-
Notifications
You must be signed in to change notification settings - Fork 4
Add mypy and black to CI #26
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
Conversation
Codecov Report
@@ Coverage Diff @@
## main #26 +/- ##
==========================================
- Coverage 98.10% 98.01% -0.09%
==========================================
Files 11 11
Lines 316 302 -14
==========================================
- Hits 310 296 -14
Misses 6 6
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
jsoules
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple minor things, mostly in tests.
mypy is complaining about three lines when I run it on command line; am I missing a config file, or is there a version inconsistency, or something? Those lines were changed to remove the ignore-type annotations, so I've noted in the review.
Also: I am getting some pylance typing warnings that aren't picked up by mypy. Dunno if we care at all, or want to address them in this PR, but just in case:
- test/models/binomial.py:
- l 48 warns that the call to
scipy.stats.beta.logpdfreturns anNDArraynot a float
- l 48 warns that the call to
- test/test_rhat.py:
- l 24 warns that subtraction isn't supported between
psij_barandpsi_barsince the former islist[floating[Any]]and the latter isfloating[Any]. The test still passes, so it's either a spurious error in pylance or something is doing some implicit munging
- l 24 warns that subtraction isn't supported between
- tests/test_metropolis.py:
- l 202, warning about inability to locate
numpy.typingwithinnumpy, which may just be PyLance being weird; however this is fixed by updating the import fromnumpy.typingto includeArrayLikeas well asNDArrayand then referring to them directly without the package qualifiers - l 150, complains that
sst.skewnorm.rvscall might be returning an int (maybe just needs a cast) - l 212 same issue as with l. 150, maybe needs a cast
- l 202, warning about inability to locate
Pylance is lying to you >>> isinstance(scipy.stats.beta.logpdf(.1, 3, 2), float)
True(it does return an array if the first argument is an array, but it isn't in this model) I think I resolved the other pylance warnings, though I'm not sure if they were also spurious |
jsoules
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This all looks good to me--nice work!
As discussed in #16, this will run
mypyandblackagainst PRs. At the moment it will fail becausemaindoesn't pass these checks, but aftermaindoes I will rebase and merge this