radio-astro-tools / spectral-cube

Library for reading and analyzing astrophysical spectral data cubes
http://spectral-cube.rtfd.org
BSD 3-Clause "New" or "Revised" License
98 stars 65 forks source link

Fix tests for numpy 2.0 #917

Closed e-koch closed 1 month ago

e-koch commented 3 months ago

Test fixes against numpy 2.0

codecov[bot] commented 3 months ago

Codecov Report

Attention: Patch coverage is 25.00000% with 6 lines in your changes missing coverage. Please review.

Project coverage is 79.93%. Comparing base (7bc6bac) to head (46e5804). Report is 5 commits behind head on master.

Files with missing lines Patch % Lines
spectral_cube/conftest.py 0.00% 2 Missing :warning:
spectral_cube/io/casa_image.py 0.00% 2 Missing :warning:
spectral_cube/analysis_utilities.py 50.00% 1 Missing :warning:
spectral_cube/spectral_cube.py 50.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #917 +/- ## ========================================== - Coverage 79.94% 79.93% -0.02% ========================================== Files 24 24 Lines 6013 6019 +6 ========================================== + Hits 4807 4811 +4 - Misses 1206 1208 +2 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

e-koch commented 3 months ago

Failures resulting from ndarray.ptp being removed in numpy >2.0 (see https://github.com/astropy/astropy/commit/fbf7c89725f34d92a9dcf739e2bea4371add7fd0)

e-koch commented 3 months ago

Removing ptp as a method fixed this issue.

Remaining issues are related to the casadata package. @keflavich the fix that was working in the uvcombine doesn't seem to be working anymore

e-koch commented 3 months ago

Updating the data manually via casaconfig seems to fix the casadata path error: https://casadocs.readthedocs.io/en/stable/notebooks/external-data.html#Populating-the-Data-Directory-Manually

e-koch commented 3 months ago

@keflavich @preshanth the tests are hitting 2 failures that seem to imply underlying changes in the image tool? I'm somewhat lost on where the change actually occurred.

The current failures have 2 sources:

  1. https://github.com/radio-astro-tools/spectral-cube/actions/runs/10256840948/job/28376751741?pr=917#step:6:933 - Creation of the multi-beam test casa image is failing to set the restoring beam: https://github.com/radio-astro-tools/spectral-cube/actions/runs/10256840948/job/28376751741?pr=917#step:6:224 Ok I solved this one by forcing the correct dtypes input to ia.setrestoringbeam
  2. The desc dict that casa-formats-io returns has a different key structure: https://github.com/radio-astro-tools/spectral-cube/actions/runs/10256840948/job/28376751741?pr=917#step:6:720. The list of keys the test is returning right now is here: https://github.com/radio-astro-tools/spectral-cube/actions/runs/10256840948/job/28376751741?pr=917#step:6:727 This issue remains
e-koch commented 3 months ago

also note that the CASA builds are pinned to numpy<2.0.

There were a bunch of additional errors related to a build numpy 1.X build somewhere in the casa module stack. That seems firmly upstream of handling in spectral-cube.

preshanth commented 3 months ago

On casa numpy issue. There is ongoing work to fix casa and put out an updated version. That should fix the numpy 2.0 problems. Will look at the dict when I get a chabce

keflavich commented 1 month ago

I'm gonna do what I suggested - being incompatible with numpy 2.0 is not ok, and no other PR tests are going to work without this.