HERA-Team / hera_qm

HERA Data Quality Metrics
MIT License
2 stars 2 forks source link

Fix some errors with coming pyuvdata changes. #439

Closed bhazelton closed 1 year ago

bhazelton commented 1 year ago

External hera_qm tests are failing on an upcoming pyuvdata PR because we are adding a warning related to future_array_shapes a long with an option to read object in using the future shape.

~I initially converted the hera_qm code to use the new option whenever a UVData/UVCal/UVFlag object is read in, but then I realized that would only work after the related pyuvdata PR is merged and would require a very recent version of pyuvdata after that. So I changed it to first ensure that hera_qm was always using future shapes (it was in most places but wasn't in a couple places, I figured those were just accidentally overlooked) and then added warnings filters for all the read calls.~

Per the discussion below, this updates the pyuvdata required version to 2.3 and always uses the future_array_shapes option when reading files into UVData/UVCal/UVFlag objects. As a part of this update, I also removed some code that is no longer needed now that pyuvdata>=2.3 is required.

I also implement some fixtures to reduce repeated code in test_xrfi.

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage: 100.00% and project coverage change: -0.02 :warning:

Comparison is base (d6c757c) 97.06% compared to head (c5529e9) 97.05%.

:exclamation: Current head c5529e9 differs from pull request most recent head 58b5f64. Consider uploading reports for the commit 58b5f64 to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #439 +/- ## ========================================== - Coverage 97.06% 97.05% -0.02% ========================================== Files 10 10 Lines 3510 3460 -50 ========================================== - Hits 3407 3358 -49 + Misses 103 102 -1 ``` | [Impacted Files](https://codecov.io/gh/HERA-Team/hera_qm/pull/439?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=HERA-Team) | Coverage Δ | | |---|---|---| | [hera\_qm/firstcal\_metrics.py](https://codecov.io/gh/HERA-Team/hera_qm/pull/439?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=HERA-Team#diff-aGVyYV9xbS9maXJzdGNhbF9tZXRyaWNzLnB5) | `93.73% <100.00%> (-0.02%)` | :arrow_down: | | [hera\_qm/omnical\_metrics.py](https://codecov.io/gh/HERA-Team/hera_qm/pull/439?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=HERA-Team#diff-aGVyYV9xbS9vbW5pY2FsX21ldHJpY3MucHk=) | `98.57% <100.00%> (+<0.01%)` | :arrow_up: | | [hera\_qm/utils.py](https://codecov.io/gh/HERA-Team/hera_qm/pull/439?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=HERA-Team#diff-aGVyYV9xbS91dGlscy5weQ==) | `97.22% <100.00%> (+0.14%)` | :arrow_up: | | [hera\_qm/xrfi.py](https://codecov.io/gh/HERA-Team/hera_qm/pull/439?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=HERA-Team#diff-aGVyYV9xbS94cmZpLnB5) | `99.59% <100.00%> (-0.02%)` | :arrow_down: | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=HERA-Team). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=HERA-Team)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

bhazelton commented 1 year ago

Looks like there's another pyuvdata PR with hera_qm errors, so I'll update this to pass on that branch as well. I'll also fix the coverage decrease.

bhazelton commented 1 year ago

Ok, this now fixes the problems in both pyuvdata PRs.

jsdillon commented 1 year ago

I'm OK with requiring the newest version of pyuvdata for the purposes of simplifying this PR (and just not merging it until that version is merged and on pypi).

bhazelton commented 1 year ago

@jsdillon Ok, I think this PR is ready. I'm not sure why the codecov check isn't reporting, but it's showing that the patch is 100% covered when you follow the codecov report link.