Skip to content

Resolve mypy error in mcmc.py #7854

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

Ok3ks
Copy link

@Ok3ks Ok3ks commented Jul 17, 2025

Description

Switching from list comprehension to np.fromiter converts the list to an NDArray which satisfies Mypy Type checks

Related Issue

  • Closes #
  • Related to #

Checklist

Type of change

  • New feature / enhancement
  • Bug fix
  • Documentation
  • Maintenance
  • Other (please specify):

📚 Documentation preview 📚: https://pymc--7854.org.readthedocs.build/en/7854/

Copy link

welcome bot commented Jul 17, 2025

Thank You Banner]
💖 Thanks for opening this pull request! 💖 The PyMC community really appreciates your time and effort to contribute to the project. Please make sure you have read our Contributing Guidelines and filled in our pull request template to the best of your ability.

@ricardoV94
Copy link
Member

Still failing: https://github.com/pymc-devs/pymc/actions/runs/16338464408/job/46163429561?pr=7854#step:5:370

You can test locally by running python scripts/run_mypy.py --verbose

@Ok3ks
Copy link
Author

Ok3ks commented Jul 17, 2025

I did and I was focused on only this pymc/sampling/mcmc.py. Is the PR to clear all mypy warnings that occur from running this syntax?

python scripts/run_mypy.py --verbose

@ricardoV94
Copy link
Member

I did and I was focused on only this pymc/sampling/mcmc.py. Is the PR to clear all mypy warnings that occur from running this syntax?

python scripts/run_mypy.py --verbose

No, just mcmc, but as you can see it still fails on the CI. Does it pass locally for you? If so, you may not be installing the same dependencies (specially numpy)

@Ok3ks
Copy link
Author

Ok3ks commented Jul 17, 2025

I've also noticed that the tests fail on the ubuntu runner. I'm unable to reproduce locally, as I use macOS

@ricardoV94
Copy link
Member

I doubt that's the reason for the difference. Type hints are not OS-specific

@Ok3ks
Copy link
Author

Ok3ks commented Jul 17, 2025

my environment has numpy 2.3.1, and I noticed requirements.txt has numpy >=1.25.0.

Is there a specific numpy version I should use?

@ricardoV94
Copy link
Member

You can check the versions the CI is installing in the log. It seems to be picking numpy 2.2.6

https://github.com/pymc-devs/pymc/actions/runs/16338464408/job/46163429561?pr=7854#step:4:22

@Ok3ks
Copy link
Author

Ok3ks commented Jul 17, 2025

I've been able to reproduce locally with numpy 2.2.6, is bumping the numpy version to 2.3.2 an option ( i understand, there's possible more to consider)

https://numpy.org/doc/stable/release/2.3.0-notes.html

@ricardoV94
Copy link
Member

I've been able to reproduce locally with numpy 2.2.6, is bumping the numpy version to 2.3.2 an option ( i understand, there's possible more to consider)

https://numpy.org/doc/stable/release/2.3.0-notes.html

We can just wait for the CI to start picking that version of numpy and the issue will auto-fix itself then? Or do we still need this change?

@Ok3ks
Copy link
Author

Ok3ks commented Jul 17, 2025

I've been able to reproduce locally with numpy 2.2.6, is bumping the numpy version to 2.3.2 an option ( i understand, there's possible more to consider)
numpy.org/doc/stable/release/2.3.0-notes.html

We can just wait for the CI to start picking that version of numpy and the issue will auto-fix itself then? Or do we still need this change?

I just checked. Yes, the change fixes the mypy error on numpy 2.3.1.

Without the change, the mypy error persists even on numpy 2.3.1

@ricardoV94
Copy link
Member

In that case we can merge the changes but leave the mcmc.py as still failing

Copy link

codecov bot commented Jul 17, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.99%. Comparing base (8a436d8) to head (8838710).
Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #7854      +/-   ##
==========================================
+ Coverage   89.25%   92.99%   +3.73%     
==========================================
  Files         108      108              
  Lines       18327    18327              
==========================================
+ Hits        16358    17043     +685     
+ Misses       1969     1284     -685     
Files with missing lines Coverage Δ
pymc/sampling/mcmc.py 91.37% <100.00%> (ø)

... and 22 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Ok3ks
Copy link
Author

Ok3ks commented Jul 18, 2025

In that case we can merge the changes but leave the mcmc.py as still failing

alright,

In that case we can merge the changes but leave the mcmc.py as still failing

I have readded "sampling/mcmc.py" to the list of failing files, now waiting on CI to pass

@Ok3ks Ok3ks marked this pull request as ready for review July 18, 2025 12:29
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.

2 participants