GFF3 translation reading via fastareader binary-search offset - #149
Open
Istar-Eldritch wants to merge 8 commits into
Open
GFF3 translation reading via fastareader binary-search offset#149Istar-Eldritch wants to merge 8 commits into
Istar-Eldritch wants to merge 8 commits into
Conversation
Rewrite GFF3TranslationReader to delegate to FastaReader with FastaSectionLocator for binary-search boundary detection. Simplifies implementation and enables efficient buffered reading of translations without loading the entire FASTA section into memory. - Changes return type from Map<String, OffsetRange> to Map<String, Long> - Reader now implements AutoCloseable for proper resource management
- Rewrite GFF3TranslationReaderTest to use Long instead of OffsetRange - Fix FastaSectionLocator binary search boundary handling for blank lines - Remove unused Mockito verification patterns - Update GFF3MapperTest for new offset API - Clean up imports and formatting
…on causes - GFF3File.write: propagate ReadException as-is (READ_ERROR=10) instead of rewrapping as WriteException; root cause is a read failure during translation lookup, not a write failure. IGFF3Feature.writeGFF3String updated to declare ReadException accordingly. - GFF3TranslationReader.readTranslation: preserve the exception cause when wrapping getSequenceSlice failures, consistent with the constructor path. - GFF3TranslationReader.readTranslationOffset: wrap FastaSectionLocator I/O failures as ReadException instead of letting UncheckedIOException escape as exit code GENERAL(1). - Document that validationEngine is retained only for constructor-signature stability; sequence-character validation now happens eagerly in FastaReader.
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.
Summary
Replaces the two hand-rolled, slow mechanisms
GFF3TranslationReaderused to locate and extract embedded##FASTAtranslations with:FastaSectionLocator— a binary search over raw file byte offsets (O(log fileSize)probes, each a small bounded forward read) that locates the FASTA boundary without ever scanning the annotation or FASTA sections in full.FastaReader(uk.ac.ebi.ena:fastareader:1.3.0) opened once at that offset, indexing in a single forward pass and extracting each translation via its bufferedgetSequenceSlice— replacing the old per-bytereadByte()extraction (the biggest real performance problem in the old code).The public
Map<String, ...>keyed by translation id is preserved; the value type changes fromOffsetRange(deleted) toLong(aFastaReadersequential entry id), threaded mechanically throughGFF3FileReader,GFF3File, andGFF3Mapper.Full design rationale, functional/non-functional requirements, and phased delivery plan:
docs/spec-gff3-translation-binary-offset.md.Behavior changes
FastaReader's protein-alphabet validation) as a hardReadException/exitREAD_ERROR=10, instead of a lazy,ValidationEngine-severity-configurable warning. This is an intentional tightening — see "Open Questions and Risks" in the spec.ReadException/exitREAD_ERROR=10(previously would have been an uncheckedRuntimeException; aWriteException/exit-11 rewrap was considered and rejected during review since the root cause is a read failure, not a write failure).Testing
FastaSectionLocatorTest(new): empty file, no-FASTA, bare-header-no-FASTA,##FASTAat byte 0 / at EOF, huge-vs-tiny section splits with a probe-count/no-full-scan assertion, mid-line vs line-boundary probes,\nvs\r\n.GFF3TranslationReaderTest: rewritten against the newreadTranslationOffset()+readTranslation(id)API, including the new eager-failure and zero-base-translation cases../gradlew spotlessCheck test --offline -Pgitlab_private_token=dummy-token: 1131/1133 passing. The 2 failures are pre-existingMainIntegrationTestcases that always fail when run as root (setReadable/setWritableare no-ops for root) — unrelated to this change.--offlineis required in this environment/sandbox only, to avoid an unrelated GitLab dynamic-dependency-version lookup returning 401 with a placeholder token; not needed with a realgitlab_private_token.Review
Went through a 2-round parent-orchestrated review (correctness, test coverage, and maintainability angles) with no blockers. Round 1 surfaced 3 mechanical consistency fixes (exception-cause preservation, locator I/O error classification, dead-field documentation) plus one exit-code semantics question, resolved and re-verified clean in round 2.