Skip to content

Read Galileo SSI VICAR files with strict=False (#219) - #220

Open
jnspitale wants to merge 1 commit into
mainfrom
gossi-vicar-strict
Open

Read Galileo SSI VICAR files with strict=False (#219)#220
jnspitale wants to merge 1 commit into
mainfrom
gossi-vicar-strict

Conversation

@jnspitale

@jnspitale jnspitale commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

Fixes #219.

Problem

vicar.VicarImage.from_file raises BARC string value is not ASCII: 'IP\x80' on GO_0002/RAW_CAL/C0003061900R.IMG. The archive label really contains a stray 0x80 byte after the BARC value, so this is a data defect, not an rms-vicar bug. A scan of all 14,672 .IMG files in the local GO_0xxx holdings found 23 affected files, all in GO_0002/RAW_CAL (2 with IP\x80, 21 with RC\x80; normal values are IP, RC, OFF).

This was already addressed on the rms-vicar side in SETI/rms-vicar#18, which added the strict flag (default True) and ships this exact file as the strict=False test case. The Cassini ISS host adopted strict=False in #154, but the GOSSI host never did.

Change

One line in oops/hosts/galileo/ssi/__init__.py: pass strict=False to VicarImage.from_file, matching the Cassini ISS host.

Verification

  • All 23 BARC-corrupted files now get past the VICAR parser; 20 load fully as Snapshots.
  • The 4 existing GOSSI gold-master tests pass.
  • The remaining 3 files, including the example in the issue, have EXPOSURE_DURATION = 0 and still fail with a ZeroDivisionError in Metronome. That is a separate, pre-existing problem affecting ~300 zero-exposure frames archive-wide (a clean zero-exposure file such as GO_0007/RAW_CAL/C0164006000R.IMG fails identically today). It will be handled in a follow-up.

No regression test is included because host tests read from the shared OOPS-Resources test-data tree and none of the affected files are there yet.

🤖 Generated with Claude Code

https://claude.ai/code/session_01PvYx8MuUbgEaoyr2DQTvPh

Summary by CodeRabbit

  • Bug Fixes
    • Galileo SSI VICAR images containing a stray non-ASCII BARC label byte can now load successfully instead of failing during strict parsing.

Fixes #219. Twenty-three files in GO_0002/RAW_CAL carry a stray 0x80
byte in the BARC label value ('IP\x80' or 'RC\x80'), which rms-vicar
rejects under its default strict parsing. rms-vicar PR #18 added the
strict=False option for exactly this file; the Cassini ISS host already
uses it, but the GOSSI host never did.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PvYx8MuUbgEaoyr2DQTvPh
@coderabbitai

coderabbitai Bot commented Sep 4, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Team

Run ID: 03dd1d22-3a74-4986-88f6-b0a768eec761

📥 Commits

Reviewing files that changed from the base of the PR and between c34c2a3 and 2c6c520.

📒 Files selected for processing (1)
  • oops/hosts/galileo/ssi/__init__.py

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


Walkthrough

The Galileo SSI from_file loader now uses non-strict VICAR parsing. This allows images with a stray non-ASCII BARC label byte to load.

Changes

Galileo SSI loading

Layer / File(s) Summary
Allow malformed BARC labels
oops/hosts/galileo/ssi/__init__.py
from_file passes strict=False when loading VICAR images and documents the malformed BARC label case.

Estimated code review effort: 1 (Trivial) | ~3 minutes

Merge Risk: ⚪ Minimal · up to 2c6c5

Galileo SSI loading now accepts VICAR files with the documented non-ASCII BARC label byte, enabling affected images to load without an identified regression risk.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the primary change: reading Galileo SSI VICAR files with strict=False. It is concise and specific.
Description check ✅ Passed The description explains the problem, linked issue, implementation, verification results, known limitations, and reason no regression test was added. It does not follow every template heading or inclu…
Linked Issues check ✅ Passed The change directly addresses issue [#219] by passing strict=False to VicarImage.from_file, allowing Galileo SSI images with non-ASCII BARC bytes to pass VICAR parsing.
Out of Scope Changes check ✅ Passed The code change is limited to the Galileo SSI file-loading path and is directly related to issue [#219]. No unrelated code changes are identified.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Sep 4, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.84%. Comparing base (c34c2a3) to head (2c6c520).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #220   +/-   ##
=======================================
  Coverage   78.84%   78.84%           
=======================================
  Files         211      211           
  Lines       25903    25903           
  Branches     2816     2816           
=======================================
  Hits        20423    20423           
  Misses       4617     4617           
  Partials      863      863           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

Bad BARC contents in some GOSSI images

1 participant