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

fix miscellaneous numpy=2.0 errors #8117

Merged
merged 15 commits into from
Sep 11, 2023
Merged

fix miscellaneous numpy=2.0 errors #8117

merged 15 commits into from
Sep 11, 2023

Conversation

keewis
Copy link
Collaborator

@keewis keewis commented Aug 28, 2023

Edit: looking at the relevant numpy issues, it appears numpy will stay a bit unstable for the next few weeks / months. Not sure how quickly we should try to adapt to those changes.

@keewis keewis added the run-upstream Run upstream CI label Aug 28, 2023
keewis added 2 commits August 28, 2023 16:28
note that with more modern versions of `numpy` the `.astype(np.str_)`
don't actually change the dtype, so maybe we can remove those.
@keewis
Copy link
Collaborator Author

keewis commented Sep 1, 2023

not sure what to do with the float_ removal, but otherwise this should be ready for review/merging. That way, at least the import check in the upstream-dev CI succeeds.

Copy link
Collaborator

@mathause mathause left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - I think this can be merged?

Isn't np.float_ and float equivalent (https://numpy.org/doc/stable/user/basics.types.html)?

@kmuehlbauer
Copy link
Contributor

kmuehlbauer commented Sep 5, 2023

Isn't np.float_ and float equivalent

Not for numpy 2.0, where it has been removed: https://numpy.org/devdocs/release/2.0.0-notes.html#numpy-2-0-python-api-removals.

Maybe just replacing np.float_ with np.float64 as suggested in the error message will fix it?

@mathause
Copy link
Collaborator

mathause commented Sep 5, 2023

Right, thanks. I found no deprecation warning or error in our upstream-dev CI, though. Maybe that's still to come? Anyway, given the description that seems to be the right approach.

Copy link
Collaborator

@headtr1ck headtr1ck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this PR.
Maybe we should add some type ignores to silence Mypy. Still not sure what the best approach is in such cases.

@dcherian dcherian added the plan to merge Final call for comments label Sep 8, 2023
@dcherian
Copy link
Contributor

dcherian commented Sep 8, 2023

Yeah we'll need to do something about the. mypy errors:

xarray/core/dataset.py:30: error: Cannot find implementation or library stub for module named "numpy.exceptions"  [import]
xarray/core/dataset.py:30: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports
xarray/tests/test_dataset.py:20: error: Cannot find implementation or library stub for module named "numpy.exceptions"  [import]
xarray/tests/test_dataarray.py:18: error: Cannot find implementation or library stub for module named "numpy.exceptions"  [import]

@keewis
Copy link
Collaborator Author

keewis commented Sep 10, 2023

Maybe just replacing np.float_ with np.float64 as suggested in the error message will fix it?

We can't, as we cannot know the default floating-point size in advance: as far as I remember, 32bit OSs (windows in particular) still have float32 as the default float. Instead, just using the python float as dtype might work?

Yeah we'll need to do something about the mypy errors:

I'd be tempted to just add numpy.exceptions.* to the list of ignored modules in pyproject.toml, but numpy does have a .pyi file for that, so not sure how to deal with the transition.

@keewis
Copy link
Collaborator Author

keewis commented Sep 10, 2023

this should be ready for merging now. Note that there are lots of failing upstream-dev mypy errors and dask seems to have some internal inconsistency in its rolling implementation, but these can be fixed / discussed in separate issues / PRs.

@mathause
Copy link
Collaborator

numpy suggests replacing np.float_ with np.float64: https://numpy.org/devdocs/release/2.0.0-notes.html

@max-sixty
Copy link
Collaborator

FYI looking through the mypy upstream errors, they're almost all matplotlib (with a couple of numpy ones)

@headtr1ck
Copy link
Collaborator

FYI looking through the mypy upstream errors, they're almost all matplotlib (with a couple of numpy ones)

See #8030

@dcherian dcherian merged commit 2951ce0 into pydata:main Sep 11, 2023
22 of 24 checks passed
@keewis keewis deleted the numpy-dtypes branch September 13, 2023 15:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AttributeError: np.unicode_ was removed in the NumPy 2.0 release. Use np.str_ instead
6 participants