radio-astro-tools / casa-formats-io

Code to handle I/O from/to data in CASA format
Other
10 stars 7 forks source link

Allow low-level reader to read all data desc ids into a list of tables #35

Closed keflavich closed 2 years ago

keflavich commented 2 years ago

@miguelcarcamov noted that one sometimes wants to read all DDIDs from a table, and looping through each DDID is quite slow because of the overhead in assembling the table metadata.

This allows the low-level reader to specify all_ddids=True and just loop over all the tables.

Example:

from casa_formats_io.casa_low_level_io.table import CASATable
rslt = CASATable.read('HD163296_continuum.ms')
tables = rslt.as_astropy_table(all_ddids=True)

This is much faster than

desc = Table.read('HD163296_continuum.ms/DATA_DESCRIPTION')
tables = [Table.read('HD163296_continuum.ms', data_desc_id=ii) for ii in desc['SPECTRAL_WINDOW_ID']]
codecov-commenter commented 2 years ago

Codecov Report

Merging #35 (adace30) into main (14c60db) will decrease coverage by 9.20%. The diff coverage is 79.31%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #35      +/-   ##
==========================================
- Coverage   56.50%   47.29%   -9.21%     
==========================================
  Files          18       18              
  Lines        2230     2235       +5     
==========================================
- Hits         1260     1057     -203     
- Misses        970     1178     +208     
Impacted Files Coverage Δ
casa_formats_io/casa_low_level_io/table.py 91.95% <78.57%> (-2.71%) :arrow_down:
casa_formats_io/glue_factory.py 65.21% <100.00%> (ø)
casa_formats_io/casa_wcs.py 22.72% <0.00%> (-75.76%) :arrow_down:
...asa_formats_io/casa_low_level_io/casa_functions.py 33.33% <0.00%> (-66.67%) :arrow_down:
casa_formats_io/casa_dask.py 43.84% <0.00%> (-51.54%) :arrow_down:
casa_formats_io/casa_low_level_io/core.py 94.73% <0.00%> (-0.66%) :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 14c60db...adace30. Read the comment docs.

keflavich commented 2 years ago

...well, I think it's faster? my tests are going slower now that I'm running timeit.

keflavich commented 2 years ago
%timeit tables = rslt.as_astropy_table(all_ddids=True)
30.4 s ± 222 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)
astrofrog commented 2 years ago

Can you rebase to resolve the conflict?

keflavich commented 2 years ago

I think I resolved them, but the rebase had several sets of conflicts that didn't make sense to me. It looked like my commits were getting rebased in the wrong order. I really have a hard time tracking rebases sometimes.

astrofrog commented 2 years ago

Not sure what happened with the rebase but it looks good now. I pushed a commit to fix the failing test which was unrelated to the changes here, not quite sure why it didn't occur before.