SETI / rms-oops

Apache License 2.0
3 stars 0 forks source link

Fixed for UVIS and VIMS #126

Closed markshowalter closed 7 months ago

markshowalter commented 7 months ago

These are fixes to UVIS and VIMS, sufficient to change the color code in Table 1, row 2 of Joe's proposal to green. There are still no unit tests but the obvious bugs were fixed and the modules now work fine according to my informal testing. They are certainly no worse than what was in there before today, so I hope they can be merged with minimal fuss.

One change that you may appreciate is that I added a new function fast_dict() in hosts/pds3. It does a quick parsing of a PDS3 label and returns a dictionary that is remarkably similar to what you expect from pdsparser.PdsLabel.from_file().as_dict(), but is 100 times faster. That's not a typo. It's a brute-force parser without using pyparsing, but not really all that complicated. It does no validation at all, so for that you still need pdsparser. There will probably be some edge cases where it can't read a label, but it appears to be quite robust in my testing so far. It is now the reader used for PDS3 labels in the VIMS and UVIS modules and it can handle the VIMS internal ISIS labels too.

codecov[bot] commented 7 months ago

Codecov Report

Attention: Patch coverage is 2.38095% with 123 lines in your changes are missing coverage. Please review.

Project coverage is 76.72%. Comparing base (277c953) to head (58e3318).

Files Patch % Lines
oops/hosts/pds3.py 2.38% 123 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #126 +/- ## ========================================== - Coverage 77.10% 76.72% -0.38% ========================================== Files 134 134 Lines 24821 24947 +126 Branches 2834 2875 +41 ========================================== + Hits 19137 19140 +3 - Misses 4829 4952 +123 Partials 855 855 ```

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

jnspitale commented 7 months ago

Thanks, that sounds great I'll update the table.