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

ci: support mypy 0.9xx #6

Closed
wants to merge 2 commits into from
Closed

ci: support mypy 0.9xx #6

wants to merge 2 commits into from

Conversation

henryiii
Copy link
Collaborator

I'd recommend running mypy from pre-commit (or maybe something like nox) as it's very important to fully control MyPy's environment, and usually nice to have it pinned as well (say with pre-commit). (See the recommendation here, for an example: https://scikit-hep.org/developer/style#type-checking )

@FFY00
Copy link
Owner

FFY00 commented Jul 13, 2021

I don't want it on pre-commit as mypy can take a while to run, but putting it on nox makes sense.

@henryiii
Copy link
Collaborator Author

You can set it to only run in the manual stage, which only runs when requested. There's an example of that under check-manifest. That way you still get the auto-updates from pre-commit.ci, for example. Feel free to do whatever you want there, though. :)

MyPy is on the very border of what I'd call a pre-commit stage check, but since failing it is invalid, I usually leave it on the normal pre-commit stage. Check-manifest is clearly on the other side of that border - slow and you very rarely ever break it. I break mypy all the time. :)

Anyway, looks like there's a small type issue. Do you have it or would you like me to fix it?

@henryiii
Copy link
Collaborator Author

Looks easy, I'll put an example fix in.

MyPy points out that the return type could be None, throwing the wrong sort of error here.
@FFY00
Copy link
Owner

FFY00 commented Sep 30, 2021

This ended up being fixed separately, but thank you so much!

@FFY00 FFY00 closed this Sep 30, 2021
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