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

NumPy 2.0 compatibility changes #224

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Conversation

JLVarjo
Copy link
Contributor

@JLVarjo JLVarjo commented Nov 13, 2024

This PR fixes some function names that were deprecated in NumPy 2.0.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@reneeotten
Copy link

@jjhelmus it would be great to get these fixes in (perhaps check the complete code base with $ ruff check path/to/code/ --select NPY201 as an additional check). Right now because numpy is not pinned to a version <2 in setup.py things break.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@JLVarjo
Copy link
Contributor Author

JLVarjo commented Nov 27, 2024

@reneeotten Thanks for the tip, ruff check found one more function name which is removed in NumPy >2.0, this is now pushed. BTW, the code should be still compatible with NumPy 1.26 after this PR, as all these renamed functions were already available in that version.

@kaustubhmote
Copy link
Collaborator

Thanks for submitting these changes. Unfortunately, the change from recfromtxt to genfromtxt will break the code in several places which need to read in strings, as the default data type is float. All calls to genfromtxt should set the kwarg dtype=None. This will make sure that the existing code still works. There are still some changes in the output record array generated by these two functions, so each example needs to be tested individually to see that it does not affect the output.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@JLVarjo
Copy link
Contributor Author

JLVarjo commented Jan 22, 2025

Added dtype to every genfromtxt call where it was missing. Any better now? I'm sorry I'm unable to test nmrglue thoroughly as I don't have most of the test data.

@kaustubhmote
Copy link
Collaborator

As I suspected, this does break some of the examples that relied on accessing elements from a record array instead of a the new array object. I will try and fix the test data story before attempting to review this again. #87

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.

3 participants