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

Remove stdatamodels dependency #91

Conversation

WilliamJamieson
Copy link
Collaborator

romancal has a dependency of stdatamodels; however, there should not be a dependency of the Roman pipeline on JWST pipeline specific packages.

The dependency is due to the stdatamodels.s3_utils module, which is a thing wrapper of the now unsupported stsci-aws-utils package. The introduction of s3_utils was largely due to an experiment to see if the jwst pipeline could load reference files from S3. While this experiment was apparently successful, it was intended to be a one-off demonstration and has been largely unused since it completed. @eslavich can provide more details. Moreover, the implantation of S3 support requires client code in jwst and romancal to know the source of input data (S3 or local filesystem) and handle the opening of internal files in a distinct fashion. Ideally the steps consuming reference files should not have to be aware of the origin of their files. Instead, CRDS, which provides these files to the steps, should handle this type of detail.

@WilliamJamieson WilliamJamieson marked this pull request as ready for review April 11, 2023 15:24
@WilliamJamieson WilliamJamieson requested a review from a team as a code owner April 11, 2023 15:24
@WilliamJamieson
Copy link
Collaborator Author

WilliamJamieson commented Apr 11, 2023

The JWST failures are expected, spacetelescope/jwst#7542 needs to be merged in order for that to pass.

@codecov
Copy link

codecov bot commented Apr 11, 2023

Codecov Report

Patch coverage has no change and project coverage change: +0.12 🎉

Comparison is base (5859cfb) 54.03% compared to head (64fd359) 54.16%.

❗ Current head 64fd359 differs from pull request most recent head bc755f6. Consider uploading reports for the commit bc755f6 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master      #91      +/-   ##
==========================================
+ Coverage   54.03%   54.16%   +0.12%     
==========================================
  Files          25       25              
  Lines        3046     3024      -22     
==========================================
- Hits         1646     1638       -8     
+ Misses       1400     1386      -14     
Impacted Files Coverage Δ
src/stpipe/config_parser.py 73.84% <ø> (+3.23%) ⬆️
src/stpipe/crds_client.py 33.33% <0.00%> (+0.77%) ⬆️
src/stpipe/extern/configobj/validate.py 67.47% <ø> (-0.23%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@WilliamJamieson WilliamJamieson merged commit cfe96d7 into spacetelescope:master Apr 14, 2023
15 checks passed
@WilliamJamieson WilliamJamieson deleted the refactor/remove_stdatamodels branch April 14, 2023 22:16
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

3 participants