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
97 stars 65 forks source link

Attempt to fix Windows build failures on CI #779

Closed keflavich closed 2 years ago

keflavich commented 2 years ago

The problems are all invocation errors: https://github.com/radio-astro-tools/spectral-cube/runs/4475860538?check_suite_focus=true#step:5:255

but it seems to be pointing at a file not closed error: https://github.com/radio-astro-tools/spectral-cube/runs/4475860538?check_suite_focus=true#step:5:153

..\..\spectral_cube\spectral_axis.py .                                   [  0%]
..\..\spectral_cube\tests\test_analysis_functions.py ................... [  1%]
                                                                         [  1%]
..\..\spectral_cube\tests\test_casafuncs.py ......ssssssssssssss         [  2%]
..\..\spectral_cube\tests\test_cube_utils.py .......                     [  2%]
..\..\spectral_cube\tests\test_dask.py ....s...E                         [  3%]
..\..\spectral_cube\tests\test_io.py .E.E.E.E.E.E.E.E.E.E.E.E.E.E.E.E.E.E [  4%]

so... let's see if we can fix it.

keflavich commented 2 years ago

There was a bug introduced in pvextractor that caused gui stuff to fail

keflavich commented 2 years ago

Unfortunately I've uncovered a real error (a regression) caused by some change in region handling. Something about compound region bboxes has changed, breaking our tests

keflavich commented 2 years ago

Some things were really broken, including one of our tests.

Both of these tests were incorrectly only getting 1 pixel along the spectral direction, but investigating in ds9 showed that 2 should've been included:

                             (('fk5.reg', (slice(None), 1, slice(None))),
                              ('fk5_twoboxes.reg', (slice(None), 1, slice(None))),

image

codecov-commenter commented 2 years ago

Codecov Report

Merging #779 (fe3c951) into master (1f67a8f) will decrease coverage by 0.04%. The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #779      +/-   ##
==========================================
- Coverage   77.86%   77.82%   -0.05%     
==========================================
  Files          24       24              
  Lines        5811     5814       +3     
==========================================
  Hits         4525     4525              
- Misses       1286     1289       +3     
Impacted Files Coverage Δ
spectral_cube/spectral_cube.py 76.65% <0.00%> (-0.16%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 1f67a8f...fe3c951. Read the comment docs.

keflavich commented 2 years ago

@astrofrog @e-koch I'm requesting reviews from both of you because I made some significant logical changes in the region handling. I think we had an uncaught error (actually an error in our tests) previously. It would be good to have an independent look at that if possible

keflavich commented 2 years ago

@e-koch you want to make those suggested changes directly on this PR?

e-koch commented 2 years ago

@keflavich Done!