limit oifits SPECTYP and TARGET to 16 characters

spacetelescope / stdatamodels

https://stdatamodels.readthedocs.io

Other

5 stars 25 forks source link

limit oifits SPECTYP and TARGET to 16 characters #281

Closed braingram closed 7 months ago

braingram commented 7 months ago

The oifits standard: https://www.aanda.org/articles/aa/pdf/2017/01/aa26405-15.pdf limits the SPECTYP and TARGET columns of OI_TARGET to 16 characters. The schema currently defines them as 32. This does not raise validation errors with oifits-check (part of oifitslib) but does raise validation errors for the online validator: http://oival.jmmc.fr/validate.xql

This PR reduces the number of characters to 16.

Checklist

[x] added entry in CHANGES.rst (either in Bug Fixes or Changes to API)
[ ] updated relevant tests
[ ] updated relevant documentation
[ ] updated relevant milestone(s)
[x] added relevant label(s)

codecov[bot] commented 7 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 64.92%. Comparing base (4d7c3a6) to head (686b58a). Report is 10 commits behind head on main.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #281 +/- ## ========================================== + Coverage 64.84% 64.92% +0.08% ========================================== Files 103 104 +1 Lines 5694 5710 +16 ========================================== + Hits 3692 3707 +15 - Misses 2002 2003 +1 ```

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

braingram commented 7 months ago

Given the scope of the changes I'm inclined to skip regtests. @hbushouse that ok with you?

hbushouse commented 7 months ago

Given the scope of the changes I'm inclined to skip regtests. @hbushouse that ok with you?

Agreed. I doubt the OIFITS stuff is covered (yet) in regtests anyway.

braingram commented 7 months ago

Thanks! I will wait for the CI to pass and merge https://github.com/spacetelescope/stdatamodels/pull/273 then fix the changelog conflict here and merge this PR.