-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Add mypy dependency and instructions when test fails #6195
Add mypy dependency and instructions when test fails #6195
Conversation
4043307
to
543c69b
Compare
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #6195 +/- ##
===========================================
+ Coverage 81.62% 93.24% +11.61%
===========================================
Files 101 101
Lines 22093 22093
===========================================
+ Hits 18034 20601 +2567
+ Misses 4059 1492 -2567
|
Hey there, just got the review request, and I'm trying to catch up on context. I managed to find #5907, and I was myself also frustrated by the slow mypy in pre-commit. If I'm understanding correctly, @michaelosthege transferred mypy to run only in the workflow. This has the disadvantage that you don't notice the mypy failure until the workflow runs. This PR addresses this by configuring the local dev environment and giving a message clarifying how to run locally. Am I understanding correctly? If so, this looks excellent. I haven't tested it locally yet, but I'll give it a shot momentarily... |
I'm traveling with my laptop and set up a fresh pymc dev environment, and ran the command. Looks like there are a bunch of errors: output:
I don't have time at the moment to investigate further, but maybe there needs to be some more configuration? |
@maresb the output you posted is actually correct.. More or less all that remains appears relevant .. |
Ah, I see now. I was in a hurry, not looking carefully enough and missed the last line "57/75 files pass as expected". My eyes were drawn to the Just a nit, but it might improve the experience to add a prominent all-caps |
That's exactly it. I should have added more context to the description |
Major / Breaking Changes
Bugfixes / New features
Docs / Maintenance