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

Fixing an issue with UVData objects being incorrectly added #1105

Merged
merged 6 commits into from
Dec 10, 2021

Conversation

kartographer
Copy link
Contributor

Fixes a bug with UVData.__add__ when objects are sorted differently.

Description

  • Made a change to UVData.__add__ that checks for the ordering of records along time-baseline, polarization, and frequency when two UVData objects have "overlap" in these domains. If there is overlap, but the ordering of the data is different, the new code will reorder the (meta-)data of one of the objects so that data are concatenated as expected.

Motivation and Context

Fixes a nasty bug were data could be concatenated incorrectly when the sorting order is different (e.g., #1102).

Closes #1102.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

Bug fix checklist:

  • My fix includes a new test that breaks as a result of the bug (if possible).
  • All new and existing tests pass.
  • I have updated the CHANGELOG.

@codecov
Copy link

codecov bot commented Dec 10, 2021

Codecov Report

Merging #1105 (e6ceacd) into main (fa00f52) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1105   +/-   ##
=======================================
  Coverage   99.81%   99.81%           
=======================================
  Files          31       31           
  Lines       14400    14422   +22     
=======================================
+ Hits        14374    14396   +22     
  Misses         26       26           
Impacted Files Coverage Δ
pyuvdata/uvdata/uvdata.py 99.93% <100.00%> (+<0.01%) ⬆️

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 fa00f52...e6ceacd. Read the comment docs.

Copy link
Member

@bhazelton bhazelton left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks for the quick fix @kartographer!

@bhazelton bhazelton merged commit 942c0b0 into main Dec 10, 2021
@bhazelton bhazelton deleted the fix_add_sorting branch December 10, 2021 22:21
@bhazelton bhazelton mentioned this pull request Feb 14, 2023
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Frequency concatenation yields wrong data
2 participants