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

Update to new pyuvdata AnalyticBeam #339

Merged
merged 6 commits into from
Nov 29, 2024

Conversation

steven-murray
Copy link
Contributor

This updates instances of AnalyticBeam to use the new class in pyuvdata.

In particular, the beams.py module is thoroughly simplified (i.e. all the poly-beam stuff). Also, the visibility simulators are simplified.

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

.squeeze()
.transpose(3, 2, 1, 0)
)
uvbeam = BeamInterface(uvbeam)
Copy link
Member

Choose a reason for hiding this comment

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

Is this really a UVBeam or can it also be an AnalyticBeam? If it can be either this name is very confusing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's true that it can be an analytic beam, but since the name is API-facing, I don't want to change it here.

@@ -1234,7 +1210,7 @@ def _handle_beam(uvbeam, **beam_kwargs):
return uvbeam
if Path(uvbeam).exists():
return UVBeam.from_file(uvbeam, **beam_kwargs)
return AnalyticBeam(uvbeam, **beam_kwargs)
raise ValueError("uvbeam has incorrect format")
Copy link
Member

Choose a reason for hiding this comment

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

same issue here and other places -- don't use uvbeam as the variable name if it can be an AnalyticBeam

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, I've gone through and tried to change as many as I could that don't affect API

setup.cfg Outdated Show resolved Hide resolved
Copy link

codecov bot commented Nov 28, 2024

Codecov Report

Attention: Patch coverage is 92.96875% with 9 lines in your changes missing coverage. Please review.

Please upload report for BASE (update-fftvis-api@89d8a71). Learn more about missing BASE report.

Files with missing lines Patch % Lines
hera_sim/sigchain.py 78.94% 1 Missing and 3 partials ⚠️
hera_sim/visibilities/matvis.py 83.33% 2 Missing and 1 partial ⚠️
hera_sim/visibilities/simulators.py 83.33% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@                 Coverage Diff                  @@
##             update-fftvis-api     #339   +/-   ##
====================================================
  Coverage                     ?   93.05%           
====================================================
  Files                        ?       25           
  Lines                        ?     3328           
  Branches                     ?      556           
====================================================
  Hits                         ?     3097           
  Misses                       ?      124           
  Partials                     ?      107           
Flag Coverage Δ
unittests 93.05% <92.96%> (?)

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.

@@ -78,7 +78,7 @@ tests =
pytest-cov>=2.5.1
uvtools
vis =
fftvis>=0.0.7
fftvis@git+https://github.com/tyler-a-cox/fftvis@mp_fftvis
line-profiler
matvis>=1.2.1
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the only thing we need to do is update the matvis version to >=1.3.0. Other than that, this all looks fantastic, thank you!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, updated.


with pytest.raises(TypeError):
beams = [BeamInterface(GaussianBeam(diameter=14.0), beam_type='power')]
print(beams)
Copy link
Member

Choose a reason for hiding this comment

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

Cruft?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, removed

@steven-murray steven-murray merged commit 5e40010 into update-fftvis-api Nov 29, 2024
24 checks passed
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.

3 participants