Skip to content
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: blt slicing and other things for new uvdata 3.0 functionality #931

Merged
merged 13 commits into from
May 24, 2024

Conversation

steven-murray
Copy link
Contributor

@steven-murray steven-murray commented Jan 31, 2024

Fixes #928

This fixes issues related to the antpair2ind update in pyuvdata v3.0. The biggest issue was that the output of antpair2ind is now no longer guaranteed to be an array of indices, but instead could be either None or a slice object. Furthermore, the results of calls to antpair2ind are now cached on the UVData object, and this cache needs to be refreshed on any update to the blt axis of the data. This is done in UVData.read() but this method is over-written in HERAData so it needed to be added manually.

Bryna's comment:
It also fixes things for some other changes in pyuvdata v3.0. There's an update to the telescope metadata handling, which has deprecation but I went ahead and fixed some of the places this comes up. Also, write_vis and write_cal broke when we actually removed support for the current array shapes. In the process of fixing these, I decided that it would be easier and better to just change them to use the .new methods on those objects. That reduces the amount of code that needs to be maintained in this repo. Note that this makes #851 unnecessary.

Copy link

codecov bot commented Feb 28, 2024

Codecov Report

Attention: Patch coverage is 88.57143% with 8 lines in your changes are missing coverage. Please review.

Project coverage is 97.18%. Comparing base (93f6fa3) to head (bbb76f5).

Files Patch % Lines
hera_cal/io.py 88.23% 8 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #931      +/-   ##
==========================================
- Coverage   97.25%   97.18%   -0.08%     
==========================================
  Files          30       30              
  Lines       10766    10733      -33     
==========================================
- Hits        10471    10431      -40     
- Misses        295      302       +7     
Flag Coverage Δ
unittests 97.18% <88.57%> (-0.08%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@jsdillon jsdillon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Basically looks good to me. Just a couple of questions.

hera_cal/io.py Outdated Show resolved Hide resolved
hera_cal/io.py Outdated Show resolved Hide resolved
@bhazelton bhazelton changed the title fix: blt slicing for new uvdata 3.0 functionality fix: blt slicing and other things for new uvdata 3.0 functionality May 22, 2024
@bhazelton bhazelton requested a review from jsdillon May 22, 2024 17:26
Copy link
Member

@jsdillon jsdillon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks ok to me!

@steven-murray
Copy link
Contributor Author

@jsdillon I'm inclined to merge even though not all lines are covered -- the remaining lines are very difficult to cover right now (most of them will be covered once pyuvdata 3 is out by default, and then we can remove some other code branches)

@jsdillon
Copy link
Member

jsdillon commented May 22, 2024 via email

@steven-murray steven-murray merged commit cc0a13d into main May 24, 2024
8 of 11 checks passed
@bhazelton bhazelton deleted the fixes-for-pyuvdata3 branch June 13, 2024 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fixes needed for future pyuvdata
3 participants