fix: adapter review findings (BrAPI/ISA-Tab/PRIDE/ENA correctness)#94
Merged
Conversation
…ation In BrAPI v2 observationLevel nests inside observationUnitPosition and block/replicate live in observationLevelRelationships, not as top-level/position keys. The mapper read them from the wrong place, so observation_level, observation_level_code, block and replicate were silently dropped for every observation unit on every conformant server; the export mirrored the mistake. The fixture had been hand-built to the buggy shape, masking it — now corrected to the real v2 structure and the tests assert the fields. Live-verified.
…tion) to_isatab passed all factors/protocols/assays/samples and study-level people/publications to every study, and hardcoded investigation contacts to empty. For a multi-study investigation this duplicated every child into every study and dropped investigation-level contacts. Partition each entity to its owning study via the dataset tree (the flat serialization carries no parent link).
The file-mapping section lacked the FMH header row and ordered FME columns as index/filename/file_type instead of the px-spec file_id/file_type/file_path, so it would not load as a real submission. Also emit lab_head_email and lab_head_affiliation (dropped before, required by px).
Pair fastq_ftp/fastq_md5 positionally (zip_longest) so an empty URL segment no longer shifts checksums. Correct the MAF docstring (m_*.tsv, not .txt) and scope the ISA-Tab writer's docstring to the metabolights codenames it actually reads (review low findings).
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
1 participant
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.
Fixes every confirmed finding from the integration-adapter code review, each with the fixture corrected to the real API shape so the tests actually guard the fix.
observationLevelis nested inobservationUnitPositionand block/replicate live inobservationLevelRelationshipsin BrAPI v2 — the mapper read them from the wrong place (silent field loss on every conformant server) and the export mirrored it. Fixed both; live-verified the test server now yields observation_level/code/block/replicate.submission.pxnow emits theFMHheader and the px column orderfile_id, file_type, file_path(was unloadable), plus lab-head email/affiliation.m_*.tsv, ISA-Tab writer scoped to the codenames it reads.Root cause throughout: fixtures encoding the mapper's assumptions, so hermetic tests passed while real data broke. 1701 tests pass; ruff / strict mypy / vulture green.
🤖 Generated with Claude Code