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

Add a UVBeam.new() method similar to the ones on UVData and UVCal #1378

Merged
merged 3 commits into from
Jan 23, 2024

Conversation

bhazelton
Copy link
Member

@bhazelton bhazelton commented Jan 22, 2024

Description

I realized that this was needed for a planned AnalyticBeam.to_uvbeam() method and decided it was cleaner to make a separate PR for this. I followed the similar UVCal and UVData methods.

In the process I also realized there were a few needed deprecations, so I:

  • Deprecated the unused spw_array and Nspws attributes on UVBeam. (Note these are currently optional, but there is no corresponding spw_ind_array attribute, so they aren't useful. I cannot see a purpose for SPW support on UVBeam objects and neither can several other people I checked in with.)
  • Deprecated the UVBeam.freq_interp_kind attribute in favor of a parameter on the interp method (similar to what was done for the interpolation_function attribute in Improve beam interp docs #1246)
  • Deprecated upper case strings in UVBeam.feed_array (these were not properly tested and never worked)

Motivation and Context

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation change (documentation changes only)
  • Version change
  • Build or continuous integration change

Checklist:

New feature checklist:

  • I have added or updated the docstrings associated with my feature using the numpy docstring format.
  • I have updated the tutorial to highlight my new feature (if appropriate).
  • I have added tests to cover my new feature.
  • All new and existing tests pass.
  • I have updated the CHANGELOG.

Breaking change checklist:

  • I have updated the docstrings associated with my change using the numpy docstring format.
  • I have updated the tutorial to reflect my changes (if appropriate).
  • My change includes backwards compatibility and deprecation warnings (if possible).
  • I have added tests to cover my changes.
  • All new and existing tests pass.
  • I have updated the CHANGELOG.

Copy link

codecov bot commented Jan 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (7790d06) 99.92% compared to head (f9942b4) 99.92%.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff            @@
##             main    #1378    +/-   ##
========================================
  Coverage   99.92%   99.92%            
========================================
  Files          36       37     +1     
  Lines       20252    20423   +171     
========================================
+ Hits        20236    20407   +171     
  Misses         16       16            
Files Coverage Δ
pyuvdata/uvbeam/beamfits.py 100.00% <100.00%> (ø)
pyuvdata/uvbeam/cst_beam.py 100.00% <ø> (ø)
pyuvdata/uvbeam/initializers.py 100.00% <100.00%> (ø)
pyuvdata/uvbeam/mwa_beam.py 100.00% <ø> (ø)
pyuvdata/uvbeam/uvbeam.py 100.00% <100.00%> (ø)
pyuvdata/uvcal/initializers.py 100.00% <100.00%> (ø)
pyuvdata/uvdata/initializers.py 100.00% <ø> (ø)

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7790d06...f9942b4. Read the comment docs.

@bhazelton bhazelton force-pushed the add_beam_initializer branch from 4861545 to a453036 Compare January 22, 2024 01:03
Deprecate the unused `spw_array` and `Nspws` attributes on UVBeam.

Deprecate the `UVBeam.freq_interp_kind` attribute in favor of a parameter on the interp method.

Deprecate upper case strings in UVBeam.feed_array
@bhazelton bhazelton force-pushed the add_beam_initializer branch from ba2d9f8 to 5d35fff Compare January 22, 2024 02:32
Copy link
Contributor

@steven-murray steven-murray left a comment

Choose a reason for hiding this comment

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

Thanks @bhazelton, this looks great (love all the typing...). Only a few easy comments to fix up.

docs/uvbeam_tutorial.rst Outdated Show resolved Hide resolved
pyuvdata/uvbeam/initializers.py Outdated Show resolved Hide resolved
pyuvdata/uvbeam/initializers.py Outdated Show resolved Hide resolved
pyuvdata/uvbeam/initializers.py Outdated Show resolved Hide resolved
pyuvdata/uvbeam/initializers.py Outdated Show resolved Hide resolved
pyuvdata/uvbeam/initializers.py Outdated Show resolved Hide resolved
@bhazelton
Copy link
Member Author

I'm also working on fixing up the pyuvsim errors, they're related to the change in handling of freq_interp_kind. I think it may actually lead to simpler code long term in pyuvsim, but short term it needs to work on different versions of pyuvdata.

Copy link
Contributor

@steven-murray steven-murray left a comment

Choose a reason for hiding this comment

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

Thanks -- yeah I think you made the right decision on the default names.

@bhazelton bhazelton merged commit fb99885 into main Jan 23, 2024
53 checks passed
@bhazelton bhazelton deleted the add_beam_initializer branch January 23, 2024 16:08
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.

2 participants