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

extend plot_main_apertures to also plot HST or Roman apertures #342

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mperrin
Copy link
Collaborator

@mperrin mperrin commented May 20, 2024

This PR updates the plot_main_apertures function to add a new parameter mission, which allows choosing to plot the JWST, HST, or Roman SIAF apertures.

pysiaf.siaf.plot_main_apertures(mission='JWST')
pysiaf.siaf.plot_main_apertures(mission='HST', fill=False)
pysiaf.siaf.plot_main_apertures(mission='Roman')

focal_plane_sizes_compared_v3

(Minor note, I don't know why for HST it defaults to filling the apertures, but for JWST and Roman it does not. In fact the plot function seems incapable of filling those apertures, regardless of whether plot=True or plot=False is set. Also, COS is not included for HST because the plot function can't handle circular apertures currently. Fixing either of those issues is out of scope for this PR however).

@mperrin mperrin requested a review from tonysohn May 20, 2024 15:18
Copy link
Collaborator

@tonysohn tonysohn left a comment

Choose a reason for hiding this comment

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

Tested successfully. I approve this change, and also agree that we should (1) get rid of filling HST apertures by default and (2) add ability to plot circular COS apertures.

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.

None yet

2 participants