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

import from the new location of normalize_axis_index if possible #8483

Merged
merged 23 commits into from
Jan 18, 2024

Conversation

keewis
Copy link
Collaborator

@keewis keewis commented Nov 25, 2023

Another one of the numpy=2.0 fixes, this time numpy.core.multiarray.normalize_axis_index has been moved to numpy.lib.array_utils (and apparently this is the first time it has been officially exposed as public API).

Since as far as I remember numpy is working on removing numpy.core entirely, we might also want to change our usage of defchararray (in the formatting tests). Not sure how, though.

@github-actions github-actions bot added the topic-arrays related to flexible array support label Nov 25, 2023
@keewis keewis added the run-upstream Run upstream CI label Nov 25, 2023
@github-actions github-actions bot added the CI Continuous Integration tools label Nov 25, 2023
@keewis
Copy link
Collaborator Author

keewis commented Nov 25, 2023

it appears we have an issue with cftime being built against numpy<2.0 but used with numpy>=2.0. Not sure if we can do anything about that, as cftime build-depends on oldest-supported-numpy, which for python=3.11 pins numpy=1.23.2.

numpy helpfully prints that we should either use wheels built against numpy>=2.0 or disable build isolation when having pip build the wheels from source. In the most recent commit I tried to do the latter, but it didn't work. If anyone has ideas how to fix this, I'm all ears (and @pydata/xarray, feel free to push directly to this branch)

@keewis
Copy link
Collaborator Author

keewis commented Nov 28, 2023

it appears the reason for the failing iris tests is cf_units and the ones for the failing backends tests are netcdf4 and h5py which would all need to be built with numpy>=2.0.0.dev0.

Does anyone have any ideas how to fix that? Otherwise we'd have to live with our upstream-dev CI failing until compatible wheels appear on conda-forge.

@keewis keewis marked this pull request as draft November 28, 2023 11:36
@dcherian
Copy link
Contributor

he ones for the failing backends tests are netcdf4 and h5py which would all need to be built with numpy>=2.0.0.dev0.

I doubt these will move before a numpy release candidate.

@keewis
Copy link
Collaborator Author

keewis commented Nov 29, 2023

I guess that means we have two choices:

  1. ignore / skip the failing tests until these packages are updated
  2. build ourselves in CI

where 2 would mean that we'd have to deal with compiling all three packages (plus make sure the cftime build from this PR works correctly)

@mathause
Copy link
Collaborator

For 2. the numpy recommendations seems to be to pass --no-build-isolation to pip but then you have to manually add all build dependencies and it does not correctly set the version number, which causes other problems. And there seems to be no way to force build dependencies (or their version) directly from pip. So I gave up on trying to get the upstream CI to work (for regionmask, but I guess this also applies here, unless I misunderstand something).

@dcherian
Copy link
Contributor

We could also run two configs: with numpy dev, without hdf,netcdf; and one with latest released numpy and dev netcdf, hdf5.

@keewis
Copy link
Collaborator Author

keewis commented Nov 29, 2023

we don't install dev netcdf4 or hdf5 / h5py (as far as I understand it, h5netcdf is pure-python), yet, so unless we change that it would become just "remove netcdf4 and hdf5 until they support numpy dev", which is much easier

@dcherian
Copy link
Contributor

Works for me, since we know numpy is making major changes.

@keewis keewis linked an issue Dec 30, 2023 that may be closed by this pull request
@keewis keewis marked this pull request as ready for review December 30, 2023 14:23
@keewis
Copy link
Collaborator Author

keewis commented Dec 30, 2023

it appears the recent commits fix most of the issues we had with the incompatibility, and we're down to ~15 failing tests now. Thus, I'll mark this as ready and I'll have this PR close the current nightly failures issue, since that has been lingering for way too long.

Edit: The failing doctest requires another warning ignore in xarray.tests

@keewis
Copy link
Collaborator Author

keewis commented Dec 30, 2023

I'll note that flox appears to be using several functions from numpy.core, as well, so we'll have to update that, too.

@keewis
Copy link
Collaborator Author

keewis commented Jan 17, 2024

mypy needs some work (probably ignores?), but otherwise this should be ready

@Illviljan
Copy link
Contributor

xarray/core/nputils.py:15: error: Cannot find implementation or library stub for module named "numpy.lib.array_utils"  [import-not-found]
xarray/core/duck_array_ops.py:43: error: Cannot find implementation or library stub for module named "numpy.lib.array_utils"  [import-not-found]
xarray/core/duck_array_ops.py:43: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports

Numpy has these odd errors sometimes, I haven't fully figured them out yet. Ignore if rephrasing the code doesn't work either.

@dcherian
Copy link
Contributor

Thanks @keewis !

@dcherian dcherian merged commit 96f9e94 into pydata:main Jan 18, 2024
26 of 29 checks passed
@keewis keewis deleted the normalize-axis-index branch January 18, 2024 16:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Continuous Integration tools run-upstream Run upstream CI topic-arrays related to flexible array support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

⚠️ Nightly upstream-dev CI failed ⚠️
4 participants