dwreeves commented on PR #31846: URL: https://github.com/apache/airflow/pull/31846#issuecomment-1595942281
@uranusjr Coming back to / checking in on this. ---- I think the main outstanding thing with this PR is in regards of what to do with default arguments in overloads. - **(1) Excluding the defaults entirely** (something you suggested) causes MyPy to fail due to incompatible typing. - The current approach includes **(2) overloaded defaults that match the actual defaults**, as per your initial request. It requires adding some `# type: ignore`s though. - Another approach that MyPy will pass is to use **(3) ellipses in overloaded defaults**. So right now we're on 2, originally I started with 3, and the latest communication I received was to switch to 1 but this causes MyPy to fail without extensive use of `# type: ignore`s. ---- An updated summary of everything else would be: - **I'm not sure if I can add additional overloads.** I could not figure out how to overload defaults with the `Literal[True]`s without MyPy describing conflicts. I imagine this is my fault for not understanding something. In any case, I think the currently proposed changes are a vast improvement over the status quo, and perhaps I or someone else can figure out the additional typing at a later date. - **I also removed some superfluous `.run()` methods in a few provider packages.** I did this to keep the code changes simpler overall, as my would have necessitated adding a ton of overloads to those files. Looking at the code and the code history of the provider package releases (for `trino`, `presto`, and `common-sql`), I do not foresee any problems with this change (also, the tests pass). But it's not literally zero risk either. And that's about it. Let me know what you want to do. 😄 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
