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

Remove some unneeded exceptions from mypy.ini #2557

Merged
merged 1 commit into from
Oct 22, 2021

Conversation

nipunn1313
Copy link
Contributor

Description

I removed these and found that mypy still passed (particularly since it's hard coded to target python 3.6 currently)

Checklist - did you ...

  • Add a CHANGELOG entry if necessary? (not needed)
  • Add / update tests if necessary? (not needed)
  • Add new / update outdated documentation? (not needed)

@JelleZijlstra JelleZijlstra added the skip news Pull requests that don't need a changelog entry. label Oct 22, 2021
@JelleZijlstra
Copy link
Collaborator

Looks good but you merge conflicted yourself :)

@ichard26
Copy link
Collaborator

ichard26 commented Oct 22, 2021

Huh, I wonder what's different this time as Łukasz had one hell of a ride trying to silence mypy for _unicodefun when we first added support for Click 8+.

@nipunn1313
Copy link
Contributor Author

My best guess is that it's because mypy.ini hard codes python 3.6?
It's also possible newer version of mypy changed something.
Doesn't feel super critical to track down.

@nipunn1313
Copy link
Contributor Author

nipunn1313 commented Oct 22, 2021

Looks good but you merge conflicted yourself :)

if only community/community#4477 - it were possible to avoid this. I prefer to make two CRs instead of one if the changes are independent. I consider it a shortcoming of github that I can't pre-handle this by stacking them.

@JelleZijlstra JelleZijlstra merged commit 62ed538 into psf:main Oct 22, 2021
@nipunn1313 nipunn1313 deleted the mypy-black branch October 22, 2021 04:09
@ichard26
Copy link
Collaborator

Doesn't feel super critical to track down.

Yeah if only. I'm working on getting black compiled with mypyc and it's failing because of the exception that we previously silenced: https://github.com/ichard26/black-mypyc-wheels/runs/4040805225?check_suite_focus=true#step:5:197

*sigh*

@nipunn1313
Copy link
Contributor Author

Dah!

Probably something like would this allow you to handle it w/o ignoring unused ignores broadly all the way across black/__init__.py.

- for module in (core, _unicodefun):
+ for module in cast(Any, (core, _unicodefun)):

^ I just confirmed this works

@nipunn1313
Copy link
Contributor Author

Put up #2573 in case you want it

@ichard26
Copy link
Collaborator

ichard26 commented Oct 29, 2021

Did you check with mypyc? I ask that because it's rather strict (makes sense as a compiler) so even mypy passing code might not work.

I can check regardless.

@nipunn1313
Copy link
Contributor Author

I did not check it. Good point about its strictness.

I suspect the old ignores were not needed w/ mypyc because the library imports were probably coming in as Any rather than types.ModuleType for mypyc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip news Pull requests that don't need a changelog entry.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants