spacetelescope / stdatamodels

https://stdatamodels.readthedocs.io
Other
5 stars 25 forks source link

New tooling for comparing the keyword dictionary and data model schemas #337

Closed braingram closed 3 weeks ago

braingram commented 1 month ago

https://jira.stsci.edu/browse/JP-3783

As it currently stands this PR:

Link to added docs: https://stdatamodels--337.org.readthedocs.build/en/337/jwst/kwtool/index.html

It would be helpful to have feedback on:

Some known issues are:

Tasks

news fragment change types... - ``changes/.feature.rst``: new feature - ``changes/.bugfix.rst``: fixes an issue - ``changes/.doc.rst``: documentation change - ``changes/.removal.rst``: deprecation or removal of public API - ``changes/.misc.rst``: infrastructure or miscellaneous change
codecov[bot] commented 1 month ago

Codecov Report

Attention: Patch coverage is 97.57174% with 11 lines in your changes missing coverage. Please review.

Project coverage is 68.93%. Comparing base (fd6be8d) to head (616a09b). Report is 8 commits behind head on main.

Files with missing lines Patch % Lines
src/stdatamodels/jwst/_kwtool/compare.py 97.10% 4 Missing :warning:
src/stdatamodels/jwst/_kwtool/kwd.py 93.61% 3 Missing :warning:
src/stdatamodels/jwst/_kwtool/__main__.py 0.00% 2 Missing :warning:
...atamodels/jwst/_kwtool/_tests/test_against_mast.py 97.36% 1 Missing :warning:
src/stdatamodels/jwst/_kwtool/_tests/test_dmd.py 93.33% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #337 +/- ## ========================================== + Coverage 66.55% 68.93% +2.38% ========================================== Files 102 114 +12 Lines 5456 5910 +454 ========================================== + Hits 3631 4074 +443 - Misses 1825 1836 +11 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

braingram commented 3 weeks ago

Based on discussion with @tapastro there are a few remaining issues to sort out with this PR prior to approval (based on discussion of the currently generated report).

braingram commented 3 weeks ago

@tapastro I think this is ready for final review. See the comment above https://github.com/spacetelescope/stdatamodels/pull/337#issuecomment-2445312775 for a checklist and linked commits for the items we discussed.

I also added a test that fetches the latest "published" keyword dictionary from MAST (using an "unofficial" service since there is no official one) and parses it generating a report as a test of the new tool. I think this is worth expanding by adding a CI job that:

This way we will know when PRs impact keyword dictionary differences (relative the published version, not the latest dev version which is not public). However it makes sense to me to split this work into a separate PR as I'd like to get this one in to also start working on PRs to address the many differences.