RAD-207: Update L1 and L2 Schema#574
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #574 +/- ##
=======================================
Coverage 96.69% 96.69%
=======================================
Files 4 4
Lines 272 272
=======================================
Hits 263 263
Misses 9 9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
schlafly
left a comment
schlafly
left a comment
There was a problem hiding this comment.
I left a comment for one missing description inline. We should develop a system for better reviewing these PRs; maybe you could post a diff between the new and old files with the PRs so that we can see only the differences? Thanks for pushing this out quickly; when the corresponding rdm changes are in I can try to get this going through romanisim.
PaulHuwe
requested review from
a team,
WilliamJamieson,
jbrookens and
kdupriestsci
as code owners
|
Regression test is running here: We expect some failures. |
| source: | ||
| origin: TBD | ||
| archive_catalog: | ||
| datatype: nvarchar(max) |
There was a problem hiding this comment.
Perhaps an archive representative could confirm if this the right datatype?
There was a problem hiding this comment.
@jbrookens ?
Note, I set this to nvarchar(max), which is used elsewhere, due to the request:
** Note for Archive Catalog / Ingest: this is potentially a very long string. Unsure of setting a character limit for this. RMO rejected the idea of using bit values and preferred a comma-separated string, e.g., "ERR1, ERR2, ERR3, ..., ERRN".
There was a problem hiding this comment.
This will need to be changed in the archive catalog as well. From JWST, we inherited a boolean, but after discussion with RMO and SE re: L0 -> L1 processing, we decided to repurpose this to be more informational than just problem = True/False.
| title: Guide Window Mode | ||
| description: | | ||
| Type of guide window mode used during the exposure. | ||
| Type of guide window mode used during the exposure. This is based on |
There was a problem hiding this comment.
I'm ok with this but did want to note that this is a change to a released schema. I highly doubt anything is depending on this description.
There was a problem hiding this comment.
RMO and SE requested we add the information to the schemas to clarify the type of information (planned vs executed). That was the motivation.
Co-authored-by: Brett Graham <brettgraham@gmail.com>
|
On the topic of improving the workflow for updating schemas in the new versioned model, what if we started each set of schema changes with a PR that does nothing but copy the existing schemas over and bump the version numbers, for schemas that will be touched? Then the git diffs would be useful? |
I opened an issue to discuss this. #577 Please feel free to add to it. If folks are available we can have a recap (perhaps once the intermediate release is out) to decide on and schedule improvements. |
Fix the docs
Fix the docs
braingram
left a comment
braingram
left a comment
There was a problem hiding this comment.
2 questions about new origin changes.
4 participants
Resolves RAD-207
Closes #561 #568
This PR adjusts several RTB directed L1 & L2 metadata schema changes.
Tasks
radtests.docs/page.no-changelog-entry-needed.)changes/:echo "changed something" > changes/<PR#>.<changetype>.rst(see below for change types).romancalregression test (https://github.com/spacetelescope/RegressionTests/actions/workflows/romancal.yml) with this branch installed ("git+https://github.com/<fork>/rad@<branch>").roman_datamodelsutilities and tests.News fragment change types:
changes/<PR#>.feature.rst: new featurechanges/<PR#>.bugfix.rst: fixes an issuechanges/<PR#>.doc.rst: documentation changechanges/<PR#>.removal.rst: deprecation or removal of public APIchanges/<PR#>.misc.rst: infrastructure or miscellaneous change