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
Modify impact_parameter() to work with N-D arrays #1604
Conversation
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.
Thank you, @pheuer! This looks really helpful.
Would you be able to add some tests of this capability? If this function gets revised in the future, I can imagine that this change could get accidentally/unknowingly reverted.
Thank you again!
Codecov Report
@@ Coverage Diff @@
## main #1604 +/- ##
=======================================
Coverage 97.21% 97.21%
=======================================
Files 83 83
Lines 7961 7961
=======================================
Hits 7739 7739
Misses 222 222
Continue to review full report at Codecov.
|
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.
It's looking close! I have a couple of minor suggestions, but we can probably merge this today.
Co-authored-by: Nick Murphy <namurphy@cfa.harvard.edu>
Those suggestions look good to me - thanks @namurphy ! |
Cool! Assuming everything passes, we can merge this just in time for the 0.8.0 release which may happen Friday we hope. |
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.
I'll enable auto-merge since as long as tests pass then this will be ready. Thanks again for doing this!
The
impact_parameter
function incollisions.py
can currently handle a 1D array of densities or temperatures, but not arrays of higher dimension. This PR changes line that handles 1D arrays to work for any shape array.impact_parameter
is called by many of the other functions in the collisions module (for example, `collision_frequency'), so this change will also enable support for higher dimension arrays in some of those functions.