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
95 stars 61 forks source link

Handle beam tables with >1 stokes component: fix for #825 #826

Closed e-koch closed 2 years ago

e-koch commented 2 years ago

This PR implements fixes #825.

codecov-commenter commented 2 years ago

Codecov Report

Merging #826 (00b32c2) into master (b4ff491) will increase coverage by 0.25%. The diff coverage is 85.71%.

@@            Coverage Diff             @@
##           master     #826      +/-   ##
==========================================
+ Coverage   77.93%   78.19%   +0.25%     
==========================================
  Files          24       24              
  Lines        5861     5920      +59     
==========================================
+ Hits         4568     4629      +61     
+ Misses       1293     1291       -2     
Impacted Files Coverage Δ
spectral_cube/io/casa_image.py 66.05% <0.00%> (ø)
spectral_cube/conftest.py 93.49% <100.00%> (+0.72%) :arrow_up:
spectral_cube/cube_utils.py 83.07% <100.00%> (+0.79%) :arrow_up:
spectral_cube/io/fits.py 86.62% <100.00%> (+0.17%) :arrow_up:
spectral_cube/spectral_cube.py 76.58% <0.00%> (+0.03%) :arrow_up:
spectral_cube/io/core.py 90.90% <0.00%> (+1.51%) :arrow_up:

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 b4ff491...00b32c2. Read the comment docs.

keflavich commented 2 years ago

LGTM. A test would probably be good to add.

Can we merge this with just FITS and move the 'todo' list to issue #825 (and reopen it)?

e-koch commented 2 years ago

I'd like to take a look on how much work the CASA image reader. It may not be too bad and, if so, would be nice to clear off in this PR. I'll update here in a few days.

e-koch commented 2 years ago

The CASA reader fix wasn't too bad.

In both cases we'll still hit issues when the components aren't IQUV, but that's a separate issue.

e-koch commented 2 years ago

Eventually we may want to move some of this into radio-beam for the polarization handling (especially any masking operations across polarizations).