-
Notifications
You must be signed in to change notification settings - Fork 21
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
Open json
files as ModelLibrary
if romancal
is installed
#389
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #389 +/- ##
==========================================
- Coverage 97.56% 97.42% -0.14%
==========================================
Files 30 37 +7
Lines 2788 3380 +592
==========================================
+ Hits 2720 3293 +573
- Misses 68 87 +19 ☔ View full report in Codecov by Sentry. |
4bc5276
to
7ac9fa6
Compare
json
files as ModelLibrary
if romancal
is installed
99aa08a
to
ad3e5db
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with one question
pytest-cov | ||
commands_pre = | ||
bash -c "pip freeze -q | grep 'roman_datamodels @' > {env_tmp_dir}/requirements.txt" | ||
pip install git+https://github.com/spacetelescope/romancal.git |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My only question is if this should be against the dev
version or the last stable version? dev
feels right, but I want to make sure you agree.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree I think dev
is most useful here since the versions tend to march along together.
Closes #267
As discussed in the issue, roman_datamodels by itself cannot open an association. So if a user provides an association to
roman_datamodels.open
we could:This PR takes the second approach. Adding support to open
.json
files provided todatamodels.open
asModelLibrary
instances (ifromancal
is installed).This required added a new CI job
with_romancal
which currently only runs the one unit test for this feature.Regtests show same failures as main: https://github.com/spacetelescope/RegressionTests/actions/runs/11000547719
Tasks
docs/
pageno-changelog-entry-needed
)changes/
:echo "changed something" > changes/<PR#>.<changetype>.rst
(see below for change types)romancal
regression test with this branch installed ("git+https://github.com/<fork>/roman_datamodels@<branch>"
)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