search

radio-astro-tools / casa-formats-io

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

Expand tests to all casadata tables #15

Closed astrofrog closed 3 years ago

astrofrog commented 3 years ago

Still a work in progress but this expands the tests to all tables in the casadata package and fixes a number of issues and implements missing features. At the time of opening this PR:

=== 57 failed, 399 passed, 1 xfailed, 168 warnings in 49.70s ===

So getting close! A number of the failures are for the same reason so I think realistically there are 5-6 separate issues that need fixing here.

codecov-commenter commented 3 years ago

Codecov Report

Merging #15 (3c3ffdb) into main (c9262a1) will increase coverage by 0.39%. The diff coverage is 96.98%.

:exclamation: Current head 3c3ffdb differs from pull request most recent head 9ffd530. Consider uploading reports for the commit 9ffd530 to get more accurate results Impacted file tree graph

@@            Coverage Diff             @@
##             main      #15      +/-   ##
==========================================
+ Coverage   95.74%   96.13%   +0.39%     
==========================================
  Files           4        4              
  Lines         940     1088     +148     
==========================================
+ Hits          900     1046     +146     
- Misses         40       42       +2
Impacted Files Coverage Δ
casa_formats_io/casa_low_level_io.py 95.90% <96.96%> (+0.59%) :arrow_up:
casa_formats_io/casa_dask.py 95.08% <100.00%> (ø)

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 c9262a1...9ffd530. Read the comment docs.

astrofrog commented 3 years ago

I'm now down to 12 failures! This is few enough that I can list and skip these in the test and work on those in a follow-up PR. The code is getting a little messy and needs some refactoring/cleanup but my main goal here is just to get things working, and a refactor can follow in a separate PR.

astrofrog commented 3 years ago

Down to two failures! One is weird - we actually read more (valid) rows than CASA, but it's not clear why CASA truncates it! The other one is a multi-dimensional column shape mismatch, probably something trivial. I can fix these in a follow-up PR though, no point in delaying merging this if CI passes.

astrofrog commented 3 years ago

Oops there is a missing commit will push later

astrofrog commented 3 years ago

@keflavich - fixed the logic issue, thanks! Let's see if CI passes now.