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

Cleanup: mypy.ini reduction #633

Merged
merged 6 commits into from
Dec 16, 2022
Merged

Conversation

jayaddison
Copy link
Collaborator

cc @sbdchd - it's totally possible that I misunderstand the reasoning behind some of the mypy.ini configuration here (maybe local development environment differences?), but either way, running mypy recipe_scrapers tests locally produces the same results before and after this change. The less configuration, the better, I reckon.

warn_return_any=False
warn_unreachable=False

strict_equality=True
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we probably want to keep most of these due to the defaults for strictness options mostly being disabled:

https://mypy.readthedocs.io/en/stable/config_file.html#confval-strict_equality

I agree it's kind of icky, but one way to think of it is explicit over implicit

another thing, they may not error now, but they will help warn in the future

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hmm, ok - understood that the checks we run could be made stricter. What would the costs and benefits of stricter checks be to our developers and maintainers?

Comment on lines -4 to -5
show_column_numbers=True
show_error_codes=True
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

and ditto for most of the others

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok, yep - enabling those seems like a good idea. Why are they disabled by default?

@jayaddison jayaddison merged commit 3ec02b5 into main Dec 16, 2022
@jayaddison jayaddison deleted the issue-593/mypi-ini-adjustments branch December 16, 2022 22:03
jayaddison added a commit that referenced this pull request Dec 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants