-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix api call to ENU_from_ECEF for pyuvdata 3.0 #402
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #402 +/- ##
==========================================
- Coverage 96.17% 96.09% -0.08%
==========================================
Files 17 17
Lines 6118 6121 +3
==========================================
- Hits 5884 5882 -2
- Misses 234 239 +5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
@jsdillon I fixed all the issues except three things:
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi!
I reviewed what @jsdillon and @steven-murray had done (thanks!) and added a few things myself that then I did not review. Note that we are dropping support for NOT future_array_shapes
(as pyuvdata
did): I removed all the tests re. the shape of the data_arrays
since they were erroring with pyuvdata
v3.0 (cannot create a UVData
object with old array shapes anymore).
Everything should be working now bar the coverage tests: a test re. line 338 of pspecdata
must be added - I did not do it as I am not familiar with the attributes in question.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
l. 338 is not tested
I removed some irrelevant test and added python 3.12 testing, but now one WF test is failing again if you want to look at it @adeliegorce. I'm not so worried about the coverage issue, but maybe @steven-murray has an idea of why pyuvdata changes have changed the coverage here? |
Hi @jsdillon, I fixed the issues in the |
I totally don't think it's worth trying to cover that one corner case exception. |
Agreed. I'll merge. |
Ah but it brought the coverage way down… |
Not according to codecov? It was just one line |
I thought I saw somewhere coverage was down to 92% but you're right, it's actually still around 96% |
No description provided.