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

Initial Association datamodel support. #101

Closed
wants to merge 2 commits into from

Conversation

PaulHuwe
Copy link
Collaborator

@PaulHuwe PaulHuwe commented Sep 7, 2022

As the title says.. this is an initial take on a data model support for associations. This is a companion ticket to the schema ticket:
spacetelescope/rad#162
I am opening this in draft PR to facilitate conversation.

@WilliamJamieson
Copy link
Collaborator

If I understand this correctly, there is some sort of json file which gets read to create the AssociationsModel? If so I don't see how this is being done because right now it looks like it will try to open one using the asdf file opening mechanism. While json should be valid yaml and most valid yaml will work as an asdf file, there are some things that asdf expects in the file in order to open it which will be missing.

@codecov
Copy link

codecov bot commented Sep 7, 2022

Codecov Report

Base: 70.31% // Head: 71.12% // Increases project coverage by +0.80% 🎉

Coverage data is based on head (985f50f) compared to base (cf63018).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #101      +/-   ##
==========================================
+ Coverage   70.31%   71.12%   +0.80%     
==========================================
  Files          15       15              
  Lines        2139     2192      +53     
==========================================
+ Hits         1504     1559      +55     
+ Misses        635      633       -2     
Impacted Files Coverage Δ
src/roman_datamodels/datamodels.py 78.44% <100.00%> (+0.18%) ⬆️
src/roman_datamodels/testing/factories.py 99.06% <100.00%> (+0.06%) ⬆️
src/roman_datamodels/testing/utils.py 99.82% <100.00%> (+<0.01%) ⬆️
src/roman_datamodels/stnode.py 90.14% <0.00%> (+0.59%) ⬆️

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 at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@PaulHuwe
Copy link
Collaborator Author

PaulHuwe commented Sep 7, 2022

The method to input via json model isn't coded yet (that is ticket RCAL-395) - there is merely a placeholder comment until that functionality is implemented.

@mairanteodoro mairanteodoro requested review from mairanteodoro and removed request for mairanteodoro February 1, 2023 15:14
mairanteodoro added a commit to mairanteodoro/roman_datamodels that referenced this pull request Feb 1, 2023
mairanteodoro added a commit that referenced this pull request Mar 1, 2023
* Initial test.

* Rename model container class; add support for list

* Refactored ModelContainer to use __getitem__ on list input.

* Added tests for ModelContainer.

* Implement additional magic methods and use temp RAD for testing.

* Temp fix for RAD.

* freeze action versions (#85)

* freeze action versions

* Apply suggestions from code review

Co-authored-by: William Jamieson <wjamieson@stsci.edu>

Co-authored-by: William Jamieson <wjamieson@stsci.edu>

* freeze action versions (#85)

* freeze action versions

* Apply suggestions from code review

Co-authored-by: William Jamieson <wjamieson@stsci.edu>

Co-authored-by: William Jamieson <wjamieson@stsci.edu>

* Revert RAD install required to default.

* Add code from PR #102 (RCAL-386).

* Remove Associations class from model registry.

* Remove ModelContainer class from model registry.

* Add Associations class to model registry and expose it.

* Add Associations class to stnode.py.

* Add empty tag attribute to Associations class.

* Change Associations._tag type from None to empty string.

* Remove __init__ method from Associations class.

* Set tag to empty string in Associations class.

* Remove Associations class from model registry.

* Expose AssociationModel and create getter/setter for Associations class.

* Change RAD lib version.

* Bug fix.

* Use temp RAD.

* Class and methods clean up.

* Remove pop method from ModelContainer.

* Fix code style issues.

* Revert RAD to main.

* Revert RAD to main.

* Remove references to Associations.

* Fix ModelContainer tests.

* Clean up.

* Code style fix.

* Fix code style issues using pre-commit hooks.

* Add Associations class & tests from PR #101.

* Point RAD to main.

* Code style fixes and class clean up.

* Add unit tests for ImageModel.

* Remove unnecessary magic methods.

* Fix issues with docstrings.

* Update docstring description.

* Remove outdated unit test.

---------

Co-authored-by: PaulHuwe <phuwe@stsci.edu>
Co-authored-by: Zach Burnett <zburnett@stsci.edu>
Co-authored-by: William Jamieson <wjamieson@stsci.edu>
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.

3 participants