-
Notifications
You must be signed in to change notification settings - Fork 667
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
histogramdd normed argument replaced by density #3976
Conversation
Codecov ReportBase: 93.67% // Head: 93.48% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## develop #3976 +/- ##
===========================================
- Coverage 93.67% 93.48% -0.19%
===========================================
Files 14 190 +176
Lines 1091 24951 +23860
Branches 0 3527 +3527
===========================================
+ Hits 1022 23326 +22304
- Misses 69 1102 +1033
- Partials 0 523 +523
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
package/CHANGELOG
Outdated
@@ -18,6 +18,8 @@ The rules for this file: | |||
* 2.5.0 | |||
|
|||
Fixes | |||
* np.histogramdd calls in :class:`DensityAnalysis` now pass the `density` |
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.
@MDAnalysis/coredevs please look at discord for this one - in my opinion we probably should do a 2.4.2 release since this has effectively killed the DensityAnalysis code. However, how we do bugfix releases really should be discussed.
TLDR; I'm pushing for us to be able to do bugfixes from a separate feature branch than develop
. That way PRs can keep rolling into develop
without breaking semver if we need to do a bugfix release.
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
So the consensus (from @orbeckst and @richardjgowers that spoke up on this), is that we'll do separate bugfix branches based off the As such, I've moved the changelog entry here to be 2.4.2, with the aim that I'll just cherry-pick the squash-merged commit out of here and re-deploy (yay..). @MDAnalysis/coredevs please review this as a priority - it's the main cause of current CI failures, and it's blocking the 2.4.x announcement (we can't announce it if it's failing as soon as it's released). |
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, lgtm
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.
LGTM
@IAlibay only concern is the 32 bit build? Causing some failures on the Azure CI |
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.
CI failure looks like h5py
not supporting NumPy 1.24.0
just yet on that platform.
32-bit Windows binaries seem to be getting dropped upstream a lot lately.
Releasing off of maintenance branches is what I do for SciPy and what Chuck does for NumPy, so +1 for that.
package/CHANGELOG
Outdated
|
||
Fixes | ||
* np.histogramdd calls in :class:`DensityAnalysis` now pass the `density` | ||
argument rather than the NumPy 1.24 removed `density` (PR #3976) |
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.
you may have meant normed
here at the end?
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
we've had this particular h5py issue for some while (pre numpy 1.24 release), but the fact that there are more now might indicate that it's either related or it has accentuated the problem. I'm going to try switching the pipeline up a little bit to limit things for 32bit. |
Only extra thing I've done is slightly change the azure pipelines to pin numpy for 32bit (hopefully this fixes it 🤞🏽 thanks @tylerjereddy !), so with two approvals I'm going to go ahead with the merge. |
* histogramdd normed argument replaced by density * pin numpy for 32bit in azure pipelines
Fixes #3975
Changes made in this Pull Request:
PR Checklist