Fix Numpy 2.4 compatibility in PDB/vectors.py#5138
Fix Numpy 2.4 compatibility in PDB/vectors.py#5138Augustin-Zidek wants to merge 4 commits intobiopython:masterfrom
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #5138 +/- ##
==========================================
+ Coverage 85.52% 86.41% +0.89%
==========================================
Files 282 282
Lines 59497 59499 +2
==========================================
+ Hits 50882 51418 +536
+ Misses 8615 8081 -534 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@mdehoon you're probably more familar with NumPy than me, does this look good to merge? Thanks! |
Bio/PDB/vectors.py
Outdated
| cosang = np.cos(angle_rads) | ||
| sinang = np.sin(angle_rads) | ||
| cosang = np.cos(angle_rads).item() | ||
| sinang = np.sin(angle_rads).item() |
There was a problem hiding this comment.
Looking at this, why not use math.sin(angle_rads) to ensure we get a plain float rather than np.float64 or similar?
|
Is this one line change not enough to fix it? |
|
Yeah, that looks reasonable as well and more minimal -- I like it. I will test that on Monday. |
|
I tested this and changing that single line is not enough, all of the changes are needed. However, instead of set_homog_trans_mtx(a1[0].item(), a1[1].item(), a1[2].item(), tm)one could do: set_homog_trans_mtx(a1[0, 0], a1[1, 0], a1[2, 0], tm)But the changes to |
|
Can you combine this then please? The |
|
All done + I also changed it to use math for trigonometric functions instead of Numpy where you suggested. This uncovered a few more issues with passing non-scalar values which I fixed. See the latest commit. |
| sinang = math.sin(angle_rads) | ||
|
|
||
| mtx[0][0] = mtx[1][1] = cosang | ||
| mtx[0][0] = cosang |
There was a problem hiding this comment.
These lines feel odd, like it was written before double indexing was possible. Does mtx[0, 0] et work here?
| set_Z_homog_rot_mtx(-sc[1], mrz) | ||
| # rotate translated a2 -polar_angle about Y | ||
| set_Y_homog_rot_mtx(-sc[2], mry) | ||
| set_Y_homog_rot_mtx(-sc[2][0], mry) |
There was a problem hiding this comment.
Looks link -sc[2, 0] should work here as well or better than -cs[2][0] does.
There was a problem hiding this comment.
Hmm, the linter is unhappy:
Bio/PDB/vectors.py: note: In function "coord_space":
Bio/PDB/vectors.py:576:26: error: Value of type "float" is not indexable
[index]
set_Y_homog_rot_mtx(-sc[2][0], mry)
^~~~~~~~
Bio/PDB/vectors.py:623:25: error: Value of type "float" is not indexable
[index]
set_Y_homog_rot_mtx(sc[2][0], mry)
^~~~~~~~
Found 2 errors in 1 file (checked 298 source files)
2 participants
I hereby agree to dual licence this and any previous contributions under both
the Biopython License Agreement AND the BSD 3-Clause License.
I have read the
CONTRIBUTING.rstfile, have runpre-commitlocally, and understand that continuous integration checks will be used to
confirm the Biopython unit tests and style checks pass with these changes.
I have added my name to the alphabetical contributors listings in the files
NEWS.rstandCONTRIB.rstas part of this pull request, am listedalready, or do not wish to be listed. (This acknowledgement is optional.)
This addresses the expired deprecation in Numpy 2.4: https://numpy.org/devdocs/release/2.4.0-notes.html#raise-typeerror-on-attempt-to-convert-array-with-ndim-0-to-scalar
Closes #5135