fix: Tecan Magellan - support compact Magellan Pro 7.5 exports - #1258
Merged
Conversation
nathan-stender
approved these changes
Aug 25, 2026
nathan-stender
added a commit
that referenced
this pull request
Aug 25, 2026
## Problem The OpenLab CDS help docs tell users they may compress their `.rslt` result set before dropping it in the File Watched directory: > (Optional) Users can additionally compress their RSLT file to a zipped file within their File Watched directory if they choose. The Agilent OpenLabs CDS connector is able to process this format along with the standard RSLT file export. That was never true in allotropy. `SUPPORTED_EXTENSIONS = "rslt"` meant a `.zip` upload was rejected before the parser saw any bytes: ``` EXPLICIT .zip FAIL: AllotropeConversionError Unsupported file extension 'zip' for parser 'Agilent OpenLab CDS', expected one of '['rslt']'. DISCOVER .zip FAIL: AllotropeVendorNotFoundError No vendor could be identified for file with extension '.zip'. ``` The `.rslt` files we already accept *are* zip archives, so the contents were never the problem — only the extension gate and, for one shape, an extra layer of nesting. ## Changes - **Accept the `zip` extension** in addition to `rslt`. - **Replace the unconditional `sniff()` with a content check.** `return True` was only safe while `.rslt` was exclusive to this parser. With `zip` added, the old sniff would have claimed every `.zip` and stolen files from Cytiva Unicorn (`"zip"`) and AppBio Absolute Q (`"csv,zip"`). It now looks for an ACAML member. - **Unwrap extra layers of compression** (`open_result_set`). Compressing the `.rslt` *folder* yields an archive whose members are the result set; compressing the `.rslt` *file* yields an archive whose only member is another archive. Both now work. - **Clearer errors.** A non-result-set archive raised bare `StopIteration`, and an uncompressed file raised `BadZipFile`. Both now raise `AllotropeConversionError` with a message, since malformed archives are newly reachable input. ## Verification Tested against archives produced by the real macOS Finder "Compress" path (`ditto -c -k --sequesterRsrc --keepParent`, which adds the `__MACOSX` resource forks that hand-built zips lack): ``` finder_folder.zip parse=IDENTICAL discover=AGILENT_OPENLAB_CDS finder_folder_renamed.rslt parse=IDENTICAL discover=AGILENT_OPENLAB_CDS finder_file.zip parse=IDENTICAL discover=AGILENT_OPENLAB_CDS finder_file_renamed.rslt parse=IDENTICAL discover=AGILENT_OPENLAB_CDS ``` IDENTICAL = byte-equal ASM against the existing fixture (~192KB of output, data cubes included), ignoring `file name` / `UNC path`, which legitimately differ. - Full suite: **1496 passed**, `hatch run lint` clean. - `tests/discover_vendor_test.py` passes, confirming no cross-vendor sniff regressions. ## Notes for reviewers - The new test **synthesizes** each compression shape from the existing `.rslt` fixture rather than checking in new files, avoiding ~3MB of duplicate test data. - Mutation-tested the new test (`MAX_ARCHIVE_NESTING` 3 → 1): exactly the two nested cases fail, so the assertions aren't vacuous. - Nesting is bounded at 3 archive levels. Beyond that a user would have to compress three or more times, which the docs don't describe. - **Still open, outside this repo:** if the connector zips the watched folder itself, a pre-zipped upload may not reach allotropy to benefit from this. The `Unsupported file extension 'zip'` error above is the signal that it did. 🤖 Generated with [Claude Code](https://claude.com/claude-code) ## Update: rebased on main (2026-08-25) Merged latest `main`, which now contains #1254 (OpenLab CDS memory + optional ACAML metadata) and #1258. Resolving the overlap in `agilent_openlab_cds_decoder.py` changed two things in this PR: - **`.sqx` stays optional.** This PR originally routed the sequence-file lookup through a new `_get_first_matching_filename` helper that raised when `.sqx` was absent. #1254 made `.sqx` optional (falling back to the ACAML method documents), and the `Sirius` fixture it added has no `.sqx`, so main's behaviour wins. The helper became dead code and was removed — the "clearer errors" bullet above now applies only to the ACAML and non-archive cases, both still covered by tests. - **`sniff` no longer copies the whole file into memory.** It now calls `get_seekable_bytes_stream()` instead of `get_bytes_stream()`. Result sets reach ~1GB, and `get_bytes_stream()` buffers the entire upload — using it in `sniff` would have silently undone #1254's memory fix on every discovery pass. Also fixed the `Quality Checks` failure that predated the merge: `from_file` returns `Mapping[str, Any]`, not `dict[str, Any]`, so the `expected` fixture's annotation failed mypy. Re-verified after the merge: **1498 passed** (up from 1496 — main added the two `Sirius` fixture cases), `hatch run lint` clean, `tests/discover_vendor_test.py` green. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Magellan Pro v7.5 can produce a compact measurement metadata block that omits the
Unit:,Number of flashes, and trailingMeas. temperaturefields expected by the Tecan Magellan parser. The parser consequently consumed the remaining metadata and raisedExpected non-null value.This change:
Testing
hatch run test tests/parsers/tecan_magellan/to_allotrope_test.py.hatch run lint.