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

Updated Roman throughput to Phase C #149

Merged
merged 3 commits into from Jul 19, 2021
Merged

Conversation

tddesjardins
Copy link
Contributor

Description

This pull request updates the documentation in Appendix B for non-HST filters for the Roman Space Telescope. The TMG and TMC files in CRDS have been updated to rename the components to Roman, as well as renaming the F146 filter to W146, and adding the new F213 filter component. The documentation has been updated in this pull request to account for these changes.

@github-actions github-actions bot added the docs label Jul 19, 2021
@@ -285,34 +285,32 @@ The Stromgren *uvby* throughputs are taken from
>>> bp = stsyn.band('stromgren,y') # doctest: +SKIP


.. _stsynphot-nonhst-wfirst:
.. _stsynphot-nonhst-roman:
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm... this will break any intersphinx linking in other repos. Do we really need to change this? It is not really visible to the user.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we should change it now rather than later. The mission has been pretty set on removing all the old references to WFIRST in favor of Roman, and I'd prefer to get things changed over earlier while it impacts few if any people rather than later when it could impact more.

Co-authored-by: P. L. Lim <2090236+pllim@users.noreply.github.com>

>>> import stsynphot as stsyn
>>> bp = stsyn.band('wfirst,wfi,f062') # doctest: +SKIP
Copy link
Contributor

Choose a reason for hiding this comment

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

Just curious... Existing usage of wfirst will just provide and error now? Or do you secretly alias it to roman for backward compatibility?

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 should provide an error that the component is not found in the TMG/TMC. All of them have been renamed to Roman.

@pllim pllim added this to the 1.2.0 milestone Jul 19, 2021
@pllim
Copy link
Contributor

pllim commented Jul 19, 2021

Need to update this too:

'https://wfirst.gsfc.nasa.gov/science/WFIRST_Reference_Information.html',

Copy link
Contributor

@pllim pllim left a comment

Choose a reason for hiding this comment

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

LGTM but let's see the rendered doc first. Thanks! 😸

@pllim
Copy link
Contributor

pllim commented Jul 19, 2021

@codecov
Copy link

codecov bot commented Jul 19, 2021

Codecov Report

Merging #149 (d7c6cdd) into master (2289bc2) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #149   +/-   ##
=======================================
  Coverage   84.87%   84.87%           
=======================================
  Files          13       13           
  Lines        1633     1633           
=======================================
  Hits         1386     1386           
  Misses        247      247           

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 2289bc2...d7c6cdd. Read the comment docs.

@pllim pllim merged commit b3dac16 into spacetelescope:master Jul 19, 2021
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.

None yet

2 participants