[MAINTENANCE] mypy config update
#6589
Conversation
✅ Deploy Preview for niobium-lead-7998 ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
👇 Click on the image for a new way to code review
Legend |
…t-type-ignore-include-an-error-code-ignore-without-code
| @@ -155,7 +168,6 @@ exclude = [ | |||
| 'rule_based_profiler/parameter_builder/value_set_multi_batch_parameter_builder\.py', # 2 | |||
| 'rule_based_profiler/parameter_container\.py', # 7 | |||
| 'rule_based_profiler/rule_based_profiler\.py', # 40 | |||
| 'tasks\.py', # Excluded from type checking the rest of the library, may be added in a separate step. | |||
There was a problem hiding this comment.
@anthonyburdi why were we ignoring this file?
I'm not getting any errors in the CI.
We should opt to suppress the specific errors if they are unresolvable rather than ignore the whole file.
| @@ -70,7 +84,6 @@ exclude = [ | |||
| 'core/usage_statistics/usage_statistics\.py', # 19 | |||
| 'core/usage_statistics/util\.py', # 2 | |||
| 'core/util\.py', # 18 | |||
| 'docs/', # Docs should not be type checked with the rest of the library. | |||
There was a problem hiding this comment.
@anthonyburdi
I moved this to the ALWAYS exclude section and tried to make be visible.
| disable_error_code = [ | ||
| # annotation-unchecked are 'warning notes', not errors and won't cause mypy to fail | ||
| # but it does create a lot of noise in the CI mypy step. | ||
| # https://mypy-lang.blogspot.com/2022/11/mypy-0990-released.html | ||
| 'annotation-unchecked' | ||
| ] |
There was a problem hiding this comment.
Just want to call out that I am globally disabling this "error" code, which we should generally never do.
But in this case, this "error" code doesn't actually result in an error anyways, so it wouldn't actually prevent any bugs without an additional configuration setting like check-untyped-defs
See the recent mypy release post section titled Warn about Variable Annotations in Unchecked Functions for these details.
If anyone has an issue with this let's discuss it.
Also, I initially turned on check-untyped-defs with the idea that if the number of errors was low, I would just fix them but... I ended up getting about 200 new errors 😬 .
I do think we should revisit this once we have better coverage of our existing "typed defs".
* develop: [FEATURE] Ensure `result_format` accessed is through Checkpoint, and warns users if `Expectation` or `Validator`-level (#6562) [BUGFIX] Remove rendered header from Cloud-rendering tests (#6597) [MAINTENANCE] Refactor `BaseDataContext` and `DataContext` into factory functions (#6531) [MAINTENANCE] Utilize a `StrEnum` for `ConfigPeer` modes (#6596) [BUGFIX] Use v3.3.6 or higher of google-cloud-bigquery (with shapely bugfix) (#6590) [MAINTENANCE] Add docs snippet checker to `dev` CI (#6594) [MAINTENANCE] Leverage `RendererConfiguration` in existing prescriptive templates (3 of 3) (#6530) [BUGFIX] Support non-string `datetime` evaluation parameters (#6571) [RELEASE] 0.15.41 (#6593) [FEATURE] ZEP - PG `BatchSorter` loading + dumping (#6580) [MAINTENANCE] Leverage `RendererConfiguration` in existing prescriptive templates (2 of 3) (#6488) [MAINTENANCE] Remove `ExplorerDataContext` (#6592) [MAINTENANCE] Small refactor of ExecutionEngine.resolve_metrics() for better code readability (and miscellaneous additional clean up) (#6587) [MAINTENANCE] `mypy` config update (#6589) [BUGFIX] convert_to_json_serializable does not accept numpy datetime (#6553) [BUGFIX] Return unique list of batch_definitions (#6579) [MAINTENANCE] typo in method name (#6585)
Changes proposed in this pull request:
pyproject.tomlmypyexclude section to clarify its purposeannotation-uncheckederror code. Add a note on variable annotation within unchecked function python/mypy#13851. Note this is not considered to be anerrorby mypy and so wouldn't actually catch things in CI without additional configuration. Removing because it creates a lot of noise in the CI.--ignore-without-codecheck which enforces that devs provide an error code along with an# type: ignorecomments.Before
After