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 tests for numpy 2.0 #917

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

Conversation

e-koch
Copy link
Contributor

@e-koch e-koch commented Aug 5, 2024

Test fixes against numpy 2.0

@e-koch e-koch changed the title remove NaNs for numpy 2.0 Fix tests for numpy 2.0 Aug 5, 2024
Copy link

codecov bot commented Aug 5, 2024

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 4 commits behind head on master.

Files Patch % Lines
spectral_cube/conftest.py 0.00% 2 Missing ⚠️
spectral_cube/io/casa_image.py 0.00% 2 Missing ⚠️
spectral_cube/analysis_utilities.py 50.00% 1 Missing ⚠️
spectral_cube/spectral_cube.py 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            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     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@e-koch
Copy link
Contributor Author

e-koch commented Aug 5, 2024

Failures resulting from ndarray.ptp being removed in numpy >2.0 (see astropy/astropy@fbf7c89)

@e-koch
Copy link
Contributor Author

e-koch commented Aug 5, 2024

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
Copy link
Contributor Author

e-koch commented Aug 5, 2024

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

@@ -105,6 +105,9 @@ def load_casa_image(filename, skipdata=False, memmap=True,
elif 'beams' in beam_:
bdict = beam_['beams']

# NOTE: temp to check failing test
print(desc['_keywords_']['coords'])
Copy link
Contributor

Choose a reason for hiding this comment

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

just commenting as a reminder since this is a temp check

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks. still trying to figure out why the dict structure seems to have changed.

@e-koch
Copy link
Contributor Author

e-koch commented Aug 5, 2024

@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
Copy link
Contributor Author

e-koch commented Aug 5, 2024

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
Copy link
Collaborator

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

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