Skip to content

Conversation

@WardBrian
Copy link
Collaborator

As discussed in #16, this will run mypy and black against PRs. At the moment it will fail because main doesn't pass these checks, but after main does I will rebase and merge this

@codecov-commenter
Copy link

codecov-commenter commented Jun 30, 2023

Codecov Report

Merging #26 (90bcde3) into main (3157c8f) will decrease coverage by 0.09%.
The diff coverage is 100.00%.

@@            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              
Impacted Files Coverage Δ
bayes_kit/__init__.py 100.00% <100.00%> (ø)
bayes_kit/autocorr.py 100.00% <100.00%> (ø)
bayes_kit/ensemble.py 100.00% <100.00%> (ø)
bayes_kit/ess.py 100.00% <100.00%> (ø)
bayes_kit/hmc.py 94.59% <100.00%> (-0.28%) ⬇️
bayes_kit/iat.py 100.00% <100.00%> (ø)
bayes_kit/mala.py 93.54% <100.00%> (-0.40%) ⬇️
bayes_kit/metropolis.py 100.00% <100.00%> (ø)
bayes_kit/rhat.py 100.00% <100.00%> (ø)
bayes_kit/smc.py 95.83% <100.00%> (-0.09%) ⬇️
... and 1 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@WardBrian WardBrian mentioned this pull request Jun 30, 2023
@WardBrian WardBrian requested a review from jsoules July 7, 2023 14:11
Copy link
Collaborator

@jsoules jsoules left a 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.logpdf returns an NDArray not a float
  • test/test_rhat.py:
    • l 24 warns that subtraction isn't supported between psij_bar and psi_bar since the former is list[floating[Any]] and the latter is floating[Any]. The test still passes, so it's either a spurious error in pylance or something is doing some implicit munging
  • tests/test_metropolis.py:
    • l 202, warning about inability to locate numpy.typing within numpy, which may just be PyLance being weird; however this is fixed by updating the import from numpy.typing to include ArrayLike as well as NDArray and then referring to them directly without the package qualifiers
    • l 150, complains that sst.skewnorm.rvs call might be returning an int (maybe just needs a cast)
    • l 212 same issue as with l. 150, maybe needs a cast

@WardBrian
Copy link
Collaborator Author

l 48 warns that the call to scipy.stats.beta.logpdf returns an NDArray not a float

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

@WardBrian WardBrian requested a review from jsoules July 7, 2023 16:02
Copy link
Collaborator

@jsoules jsoules left a 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!

@WardBrian WardBrian merged commit d32dcb5 into main Jul 7, 2023
@WardBrian WardBrian deleted the ci/mypy-black branch July 7, 2023 16:16
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.

4 participants