Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MAINTENANCE] mypy config update #6589

Merged
merged 22 commits into from Dec 15, 2022
Merged
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
8f23203
add variable tuple types
Kilo59 Nov 16, 2022
b2d82cd
ignore some table_metrics issues
Kilo59 Nov 16, 2022
72ee14c
Merge remote-tracking branch 'origin/develop' into develop
Kilo59 Nov 16, 2022
3c3243a
Merge remote-tracking branch 'origin/develop' into develop
Kilo59 Nov 16, 2022
f71e3fb
Merge remote-tracking branch 'origin/develop' into develop
Kilo59 Nov 17, 2022
9d55f77
Merge remote-tracking branch 'origin/develop' into develop
Kilo59 Nov 17, 2022
57f1570
Merge remote-tracking branch 'origin/develop' into develop
Kilo59 Nov 17, 2022
b434b6a
Merge remote-tracking branch 'origin/develop' into develop
Kilo59 Nov 18, 2022
08a1671
missing comma `,` in requirements
Kilo59 Nov 18, 2022
6deaa9b
Merge remote-tracking branch 'origin/develop' into develop
Kilo59 Nov 18, 2022
8e89d7b
Merge remote-tracking branch 'origin/develop' into develop
Kilo59 Nov 18, 2022
929b682
Merge remote-tracking branch 'origin/develop' into develop
Kilo59 Nov 18, 2022
8b3a119
Merge remote-tracking branch 'origin/develop' into develop
Kilo59 Nov 19, 2022
ef6c84c
Merge remote-tracking branch 'origin/develop' into develop
Kilo59 Nov 20, 2022
28190a4
Merge remote-tracking branch 'origin/develop' into develop
Kilo59 Nov 21, 2022
4a10adc
Merge remote-tracking branch 'origin/develop' into develop
Kilo59 Dec 14, 2022
a770046
Merge remote-tracking branch 'origin/develop' into develop
Kilo59 Dec 15, 2022
440efc1
move `docs` ignore to `ALWAYS EXCLUDE` section
Kilo59 Dec 15, 2022
7fa6500
don't ignore `tasks.py`
Kilo59 Dec 15, 2022
d228c7c
disable the `annotation-unchecked` warnings
Kilo59 Dec 15, 2022
80379aa
remove comma
Kilo59 Dec 15, 2022
d028e24
https://mypy.readthedocs.io/en/stable/error_code_list2.html#check-tha…
Kilo59 Dec 15, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 15 additions & 3 deletions pyproject.toml
Expand Up @@ -25,10 +25,24 @@ ignore_missing_imports = true
follow_imports = 'silent'
warn_redundant_casts = true
show_error_codes = true
enable_error_code = [
'ignore-without-code'
]
# The following list of codes are globally ignored, do not add to this list
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'
]
Comment on lines +32 to +37
Copy link
Member Author

@Kilo59 Kilo59 Dec 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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".

exclude = [
# If pattern should always be excluded add comment explaining why
# BEGIN ALWAYS EXCLUDE SECTION #####################################################
# If pattern should always be excluded add comment explaining why and put
'docs/', # Docs should not be type checked with the rest of the library.
'_version\.py', # generated by `versioneer`
'v012', # legacy code
# END ALWAYS EXCLUDE SECTION ######################################################
#
# #################################################################################
# TODO: complete typing for the following modules and remove from exclude list
# number is the current number of typing errors for the excluded pattern
Expand Down Expand Up @@ -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.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@anthonyburdi
I moved this to the ALWAYS exclude section and tried to make be visible.

'dataset/sparkdf_dataset\.py', # 3
'dataset/sqlalchemy_dataset\.py', # 16
'datasource/data_connector/configured_asset_sql_data_connector\.py', # 47
Expand Down Expand Up @@ -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.
Copy link
Member Author

@Kilo59 Kilo59 Dec 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

'validation_operators/types/validation_operator_result\.py', # 35
'validation_operators/validation_operators\.py', # 16
'validator/exception_info\.py', # 1
Expand Down