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

Removed **kwargs from sample_numpyro_nuts and sample_blackjax_nuts #6768

Merged
merged 1 commit into from
Jun 10, 2023

Conversation

fonnesbeck
Copy link
Member

@fonnesbeck fonnesbeck commented Jun 10, 2023

What is this PR about?

The **kwargs for sample_numpyro_nuts and sample_blackjax_nuts are not used by the functions and can cause hard-to-detect bugs when keyword arguments are passed but not used. With this change, unused arguments will generate an exception.

Checklist


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

@fonnesbeck fonnesbeck requested a review from ricardoV94 June 10, 2023 03:36
@canyon289
Copy link
Member

This ones bit me before

@codecov
Copy link

codecov bot commented Jun 10, 2023

Codecov Report

Merging #6768 (846578c) into main (864ecb3) will increase coverage by 0.01%.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #6768      +/-   ##
==========================================
+ Coverage   91.91%   91.93%   +0.01%     
==========================================
  Files          95       95              
  Lines       16134    16134              
==========================================
+ Hits        14830    14833       +3     
+ Misses       1304     1301       -3     
Impacted Files Coverage Δ
pymc/sampling/jax.py 98.26% <ø> (ø)

... and 1 file with indirect coverage changes

@fonnesbeck
Copy link
Member Author

fonnesbeck commented Jun 10, 2023

Yes, just spent almost a week chasing down an issue that was caused by my passing postprocesing_chunks (note missing "s") to sample_numpyro_nuts, which it silently accepts but does nothing with.

You would think that all the fine pre-commit hooks we have would object to allowing unused kwargs in a function's signature, but apparently that's totally cool.

@fonnesbeck fonnesbeck merged commit 765fdef into pymc-devs:main Jun 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants