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.

Sign up for GitHub

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

Jump to bottom

Rcal 740 Add Data Products documentation to RTD #1064

Merged
merged 14 commits into from
Jan 18, 2024
Merged

Rcal 740 Add Data Products documentation to RTD #1064

merged 14 commits into from
Jan 18, 2024

Conversation

ddavis-stsci
Copy link
Collaborator

Resolves RCAL-740

Closes #1061

This PR adds documentation for the science products in the exposure level pipeline.

Checklist

  • [N/A] added entry in CHANGES.rst under the corresponding subsection
  • [N/A] updated relevant tests
  • updated relevant documentation
  • updated relevant milestone(s)
  • added relevant label(s)
  • [N/A] ran regression tests, post a link to the Jenkins job below. How to run regression tests on a PR

@ddavis-stsci ddavis-stsci added documentation Improvements or additions to documentation no-changelog-entry-needed labels Jan 17, 2024
@ddavis-stsci ddavis-stsci added this to the 24Q2_B13 milestone Jan 17, 2024
@ddavis-stsci ddavis-stsci self-assigned this Jan 17, 2024
@ddavis-stsci ddavis-stsci requested a review from a team as a code owner January 17, 2024 13:27
@ddavis-stsci ddavis-stsci requested a review from nden January 17, 2024 13:27
@codecov Codecov
Copy link

codecov bot commented Jan 17, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (46535ba) 76.72% compared to head (4c56a96) 76.72%.
Report is 1 commits behind head on main.

❗ Current head 4c56a96 differs from pull request most recent head 66a62ed. Consider uploading reports for the commit 66a62ed to get more accurate results

Additional details and impacted files 
@@           Coverage Diff           @@
##             main    #1064   +/-   ##
=======================================
  Coverage   76.72%   76.72%           
=======================================
  Files         105      105           
  Lines        7015     7015           
=======================================
  Hits         5382     5382           
  Misses       1633     1633
Flag Coverage Δ *Carryforward flag
nightly 63.02% <ø> (ø) Carriedforward from 46535ba

*This pull request uses carry forward flags. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

nden
nden requested changes Jan 17, 2024
View reviewed changes
Copy link
Collaborator

@nden nden left a comment

Choose a reason for hiding this comment

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

I left some inline comments.
In addition the Exposure level products say the data is a 4D array. This needs to be corrected.

docs/index.rst Outdated
Welcome to the documentation for the Roman calibration software,
`romancal`. This package contains the Python software suite for the
Roman Space Telescope (RST) calibration pipeline, which processes data
from all Roman instruments by applying various corrections to produce
Copy link
Collaborator

Choose a reason for hiding this comment

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

Only WFI, not all instruments

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed.

docs/index.rst Outdated
@@ -12,6 +12,24 @@ The Roman Space Telescope Calibration Pipeline
:align: center
:alt: Nancy Roman Space Telescope

Welcome to the documentation for the Roman calibration software,
`romancal`. This package contains the Python software suite for the
Copy link
Collaborator

Choose a reason for hiding this comment

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

romancal is written as a link but is not actually linked.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed the link.

docs/index.rst Outdated
individual exposures as well as high-level data products (mosaics,
catalogs, etc.). The tools in this package allow users to run and
configure the pipeline to custom process their Roman data.
Additionally, the `romancal` package contains the interface to
Copy link
Collaborator

Choose a reason for hiding this comment

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

Again romancal is in back ticks, which attempt to create a link but the link is not provided.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed.

CHANGES.rst Outdated
@@ -17,8 +17,6 @@ jump detection
documentation
-------------

- Update jump step docs [#1035]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Was this change intended?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This went in Dec 7, I think I just moved the text.
We used to keep the change log in alphabetical order. I think I'll just give up.

@ddavis-stsci ddavis-stsci force-pushed the rcal-740 branch 2 times, most recently from 84b0cf9 to 6a9fa3f Compare January 17, 2024 20:46
nden
nden reviewed Jan 17, 2024
View reviewed changes
docs/roman/data_products/science_products.rst Outdated
+--------------+----------+------------+-------+-------------------------------+
| data array | | Data Type | Units | Dimensions |
+==============+==========+============+=======+===============================+
| data | Required | uint16 | DN | ncols x nrows x nresultants |
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since this is using numpy arrays directly in Python, as opposed through FITS, should the dimensions be listed as
nresultants x nrows x ncols?

Copy link
Collaborator

Choose a reason for hiding this comment

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

If this is the right way to present the dimensions then they need to change in every row in this table.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated

nden
nden approved these changes Jan 17, 2024
View reviewed changes
Copy link
Collaborator

@nden nden 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!

@ddavis-stsci ddavis-stsci merged commit a45c47d into spacetelescope:main Jan 18, 2024
28 checks passed
@ddavis-stsci ddavis-stsci deleted the rcal-740 branch January 18, 2024 12:45
stscieisenhamer pushed a commit to stscieisenhamer/romancal that referenced this pull request Jan 19, 2024
@ddavis-stsci @zacharyburnett
@schlafly @pre-commit-ci @stscieisenhamer
Rcal 740 Add Data Products documentation to RTD (spacetelescope#1064)
bddbf89 
Co-authored-by: Zach Burnett <[email protected]>
Co-authored-by: Eddie Schlafly <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nden nden nden approved these changes

Assignees

@ddavis-stsci ddavis-stsci

Labels
documentation Improvements or additions to documentation no-changelog-entry-needed Wide Field Instrument (WFI)
Projects
None yet
Milestone
24Q2_B13
Development

Successfully merging this pull request may close these issues.

Add documentation for data product types to rtd
3 participants
@ddavis-stsci @nden @zacharyburnett