Ingest ENCODE annotation files during sync — Closes #94 - #108
Draft
conradbzura wants to merge 12 commits into
Draft
Ingest ENCODE annotation files during sync — Closes #94#108conradbzura wants to merge 12 commits into
conradbzura wants to merge 12 commits into
Conversation
Both formats pair two loci per record, and both were aliased onto the EDAM term for the single-interval format they resemble -- bedpe to BED, bigInteract to bigBed. Processor lookup keys on the format name, so that alias handed them to the BED tabix pipeline, which sorted and indexed the first locus of each record and left the second unindexed. The result was a cached artifact that looked successful and was wrong, and the byte-sniff guard cannot catch it because both are plaintext or gzip. EDAM has no term for either, verified against OLS4, so the tokens are minted under a cfdb: prefix rather than borrowed. The distinct name is what does the work: neither appears in any processor's supported_formats, so a data request now streams the raw upstream file and an index request fails cleanly, instead of serving something silently incorrect. This applies to the .bedpe already published under type=Experiment, not only to the annotation files that prompted it. Those files lose an index they should never have had; tileset endpoints that understand paired intervals are planned separately.
These five values are the complete Output type domain of the two annotation types the sync ingests, verified against the live metadata TSVs, and none of them appeared in the table. Without them every one of the ~29,000 annotation files would be stored with a null data_type. The element and gene link types resolve to the generic data term rather than to a features or track term. EDAM has nothing for a predicted regulatory relationship between two loci, and the pre-existing chromatin interactions entry already made that call the same way. A deliberately vague term is preferable to claiming the files are something they are not.
Eight fields the annotation metadata TSV publishes and the experiment one does not: annotation_type and organism on the file, annotation_type, software_used and encyclopedia_version on the dataset, and life_stage, age and age_units on the biosample. annotation_type is held on the file as well as the dataset. The dataset is where the property belongs, but filtering files by it is the actual use case, and routing that through a collection subdocument on every query is not worth avoiding one duplicated string. The age fields stay strings rather than becoming a number. The released corpus contains "2-4" and "unknown" alongside decimals, and it distinguishes "10.5" from "10.50". That is also why they do not go to Subject.age_at_sampling, which is a float in years and could represent neither the ranges nor the sentinels -- and an annotation row names no donor to build a Subject from in any case. The output types are derived from the models, so the SDL gains all eight on both the input and the output side.
Everything ENCODE publishes as an Annotation -- candidate cis-regulatory elements, element gene regulatory interaction predictions, chromatin state, footprints -- was absent from cfdb, which fetched only type=Experiment. Annotation files are the interpretive layer over the raw experiments: a cCRE track is what a Gosling visualization usually wants alongside signal. The streaming loop is extracted so both TSVs share it, and each stream now names itself in its log lines. Which annotation types are ingested is read from ENCODE_ANNOTATION_TYPES rather than hardcoded: the full space is 580,910 datasets, 86% of it footprints, so the allowlist has to exist from the outset rather than be retrofitted once volume becomes a problem. Entries are trimmed and deduplicated, since files are written with insert_many into a collection with no unique key and a repeated token would load a whole type twice. One request per type, so a failure or a gateway timeout on one costs only that type. The two TSVs do not share a column set -- Annotation publishes 32 columns to Experiment's 59, six of them renamed. Rather than a second transformation, the annotation columns are renamed to their experiment equivalents and run through the shared one, so a single mapping serves both and a column the TSV omits comes out unset instead of derived from something else. That is how an annotation document ends up with no subjects: there is no Donor(s) column to build one from, and inventing a donor would be worse than admitting there is none. Two deliberate departures from the experiment path. The dataset collection is built from the accession alone rather than requiring a biosample term, because 48 released cCRE files name no biosample and gating on one would leave 24 dataset accessions unqueryable. And the collection's persistent_id points at /annotations/, since ENCODE serves the two dataset kinds under different paths. Also populates extra.encode.assembly, which the schema has always published and nothing ever wrote -- a client filtering on it matched nothing at all. Multi-assembly cCREs, spanning GRCh38, mm10 and hg19, are what make that worth fixing now.
The sync now runs one phase for released experiments plus one per configured annotation type. A phase that raises is logged and recorded and the remaining phases still run, so a transient failure on one stream does not cost the corpus the others. The task still fails at the end naming the phases that broke -- reporting a clean sync over a partially loaded collection would be worse than a visible failure. Getting that right needed more than a try/except around each phase. A stream can die mid-flight, which is exactly what an asyncio.TimeoutError against the metadata budget does, and reporting a phase's contribution only on its clean return meant a failed phase contributed nothing to the total even though its completed batches were already committed. The tallies, mutated per row, kept counting the rows the partial batch then discarded. Three numbers in one log block disagreeing with each other and with the database. The row count is now reported by mutating an accumulator and the trailing batch is flushed in a finally, so a partial load reports itself accurately. Isolating phases also means abandoning a generator mid-iteration on every failure. The stream owns an aiohttp session inside its own context manager, released only when the generator is finalized, so the streams are closed explicitly rather than left to the garbage collector -- with one stream per sync that was academic, with N+1 it is not. The annotation tally is seeded from the configured allowlist so a type ENCODE stops publishing reports zero rather than vanishing from the log. An absent key is what nobody notices; a zero is what everybody does. The broad except stays narrower than BaseException on purpose: a CancelledError caught there would be logged as a phase failure and followed by every remaining phase, which is the opposite of cancelling.
annotation_type is the field the whole annotation corpus is meant to be reached through, and organism is what narrows a result set spanning human and mouse. Both needed two things the schema alone does not give them. An index, because the ENCODE sync writes the files collection directly and never reaches the Rust materializer that owns the rest of its keys -- so nothing else would create these, and the flagship filter would be a full scan of a ~300,000 document collection on an unauthenticated endpoint. A place in the distinct-values allowlist, because filtering by annotation type is only useful if the vocabulary is discoverable. Otherwise a client has to already know ENCODE's exact spelling, which is the string matching the annotation ingest set out to replace. Both are small closed vocabularies, which is what that allowlist is for -- unlike the accessions, which are excluded from it deliberately because enumerating one would return the whole corpus.
The module docstring is the reference for this service and described only half of it once the annotation path landed. It now carries the second metadata URL, the six renamed columns, the seven annotation-only ones, and -- as importantly -- the list of columns the annotation TSV does not publish, which are left unset rather than derived. The supplement gains the same mapping, the allowlist and its configured types with their real dataset and file counts, the two minted format terms and why they are minted, the annotation output types, and a note on how phase isolation behaves when a stream dies partway. Two things are called out in both places because they are the kind of detail a reader would otherwise have to reconstruct from the code: that an annotation document carries no subjects, since the TSV names no donor to build one from, and that its dataset is built from the accession alone so that the 24 datasets whose files name no biosample stay queryable.
The minted prefix is the mechanism that stops an unrepresented format being aliased onto a term meaning something else, and nothing tied the exported constant to the table it governs. Three property tests do that: every id is either an EDAM term or a minted one, a minted id is derived from its own key, and a minted name is shared with no EDAM entry and claimed by no processor. That last one is the load-bearing case -- since routing keys on the name, a minted id paired with the name "BED" would drop the file straight back into the pipeline the minting exists to keep it out of. The routing assertion now goes through a registry wired the way the API wires it, rather than two hand-picked classes, so it tracks what is actually served. A positive control pins that plain BED still reaches the tabix processor, since "no processor claims it" would also hold if lookup were simply broken. The annotation output types are asserted against their exact terms. The previous test only checked for a non-null data term, so all five could have pointed at the wrong one and passed.
The fixture carries the full 32-column annotation header with values from a real released cCRE file, so the tests exercise the shapes the corpus actually contains rather than an idealized row: an empty Assay term name, which every one of the 12,448 cCRE rows has, a multi-valued Targets, and a non-human organism. On the fetch side: the URL is asserted by parsing its query rather than by substring, which would pass against a stray or duplicated parameter, and a type containing URL metacharacters is pinned as unable to smuggle one in. Each failure mode -- HTTP status, timeout, network error -- is checked to name its own stream, which is the entire reason the label was threaded through, and to release its session. That last one caught a real leak: async for does not close the iterator it abandons, so the delegating wrapper left the session-owning inner generator suspended. Allowlist parsing gets its adversarial cases: all-blank values disable ingest rather than widening it to an unfiltered query, repeats collapse, and a property test pins that no configuration at all can produce a blank entry. The default-when-unset test now deletes the variable first -- it previously asserted against whatever the ambient environment held and passed with an override exported. On the transform side: the renames, the annotation-only fields, the absence of the experiment-only ones, and the two behaviors that differ by design -- no subjects without a donor column, and a dataset built from the accession alone. The experiment path gets the pins it was missing: its persistent_id, which was asserted nowhere in the repository despite the URL path becoming a caller-supplied argument, and anatomy, which the diff's guard change touched and no test read. One document is validated through FileMetadataModel end to end. The ingest writes plain dicts and the resolver reads them back through the model on every row, so a key the model does not declare is dropped silently -- the filter would exist and match nothing.
The central pin is that the reported total always equals the documents actually committed. It is asserted for a stream that dies mid-flight -- the shape a metadata timeout takes -- and then generalized by a property test over arbitrary fan-out, row counts, batch sizes and failure patterns, since the example cases only fix a handful of shapes. Around it: the partial batch of a failed phase is committed rather than discarded, a repeated allowlist entry loads its type once, a configured type that returns nothing is reported as zero, a cancellation propagates instead of being swallowed as a phase failure, and the error chains the first failure so the traceback points at what started the cascade. The total-failure path is pinned too, since clearing before loading means every phase failing leaves the collection empty. Batching gets its boundaries -- exactly one batch, one plus a remainder, and none -- plus the skip path for a row that does not transform. Note that the ingest clears the same list it hands to insert_many, so these assert on call counts and the resulting documents, never on recorded call arguments, which read back empty. The fixture that disables annotation phases moves from module scope to the classes that drive the sync. It governs a network boundary -- without it a unit test reaches the real portal once per configured type -- so it belongs next to the tests that depend on it rather than silently over the 4DN and HuBMAP ones. A new test opts out of it deliberately, because nothing otherwise joined the default allowlist to the phases that run.
A filter is only useful if the path it flattens to is the path the ingest writes. A mismatch there would leave the whole annotation corpus unfilterable while every ingest test still passed, so each of the eight new fields is asserted from the GraphQL input through to its dotted predicate -- including the five-segment biosample path, which crosses two array levels and is the one most easily got wrong. A property test pins that none of them is folded the way accessions are. ENCODE's vocabulary is case- and space-significant, so folding any of them would make the documents permanently unmatchable with nothing raising. The end-to-end tests then ask the questions a client would: give me the cCRE files, narrow them to mouse, narrow them to mm10, and read the fields back out. The distinct-values test pins that the annotation types can be enumerated, since filtering by one is only useful if the vocabulary is discoverable. The index spec test moves from exact equality to containment and gains a sibling for the annotation keys, so the accession claim stays readable now that the list covers more than accessions.
The point of giving bedpe and bigInteract their own format terms is what the routes do with them, and that was asserted only at the mapping table. Both routes are now pinned against a registry wired the way the API wires it: a data request reaches the direct streaming path without dispatching a workflow, and an index request returns 404 without consulting the executor at all. Serving the raw file is the correct outcome here rather than a degradation. The alternative these tests rule out is the previous behavior, where the pipeline claimed the file and committed an index built from the first locus of each record.
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
Add a second ingest path for
type=Annotation&status=released, so the interpretive layer over the raw experiments cfdb already serves becomes reachable. A cCRE track is what a Gosling visualization usually wants alongside signal, and none of it was queryable before.Which annotation types are ingested is configuration rather than a hardcoded assumption. ENCODE publishes 580,910 annotation datasets and 86% of them are footprints, so the allowlist exists from the outset rather than being retrofitted once volume becomes a problem. The default is the two types the issue names: 7,773 datasets, 29,140 files.
The two TSVs do not share a column set — Annotation publishes 32 columns to Experiment's 59, six of them renamed. Rather than a parallel transformation, the annotation columns are renamed to their experiment equivalents and run through the shared one, so a single mapping serves both and a column the TSV omits comes out unset instead of derived from something else. That is how an annotation document ends up with no subjects: there is no
Donor(s)column to build one from, and inventing a donor would be worse than admitting there is none.The sync now runs one phase per stream, isolated so a failure in one does not cost the corpus the others, while still failing the task and naming what broke.
Verified against the live corpus rather than only against fixtures: all 29,140 real rows from both annotation TSVs transform and validate through
FileMetadataModelwith zero failures, and every field the issue requires is populated on every document.Closes #94
Proposed changes
Ingest released annotations per configured type
Extract the streaming loop so both TSVs share it, and give each stream a label it carries into its log lines — with several streams per sync, a timeout that does not say which one died is not actionable.
ENCODE_ANNOTATION_TYPESbounds the allowlist; entries are trimmed and deduplicated, since files are written withinsert_manyinto a collection with no unique key and a repeated token would load a whole type twice. One request per type, so a failure or a gateway timeout on one costs only that type.Two departures from the experiment path are deliberate. The dataset collection is built from the accession alone rather than requiring a biosample term, because 48 released cCRE files name no biosample and gating on one would leave 24 dataset accessions unqueryable. And its
persistent_idpoints at/annotations/, since ENCODE serves the two dataset kinds under different paths.Run the ingest as isolated per-stream phases
A phase that raises is logged and recorded and the remaining phases still run. The task still fails at the end naming the phases that broke — reporting a clean sync over a partially loaded collection would be worse than a visible failure.
A stream can die mid-flight, which is what an
asyncio.TimeoutErroragainst the metadata budget does, so a phase reports the rows it committed rather than only those from a clean return, and its trailing partial batch is flushed rather than discarded. The streams are closed explicitly rather than left to the garbage collector, since isolating phases means abandoning a generator that owns an HTTP session on every failure. The annotation tally is seeded from the allowlist so a type ENCODE stops publishing reports zero rather than vanishing from the log.Give bedpe and bigInteract their own file_format terms
Both formats pair two loci per record and both were aliased onto the EDAM term for the single-interval format they resemble. Since processor lookup keys on the format name, that alias handed them to the BED tabix pipeline, which indexed the first locus of each record and left the second unindexed — a cached artifact that looked successful and was wrong, and one the byte-sniff guard cannot catch because both are plaintext or gzip.
EDAM has no term for either, verified against OLS4, so the tokens are minted under a
cfdb:prefix rather than borrowed. Neither name appears in any processor'ssupported_formats, so a data request now streams the raw upstream file and an index request returns 404.Make the annotation fields queryable
Eight new fields across the three enriched ENCODE models, mirrored onto the GraphQL inputs.
annotation_typeis held on the file as well as the dataset: the dataset is where the property belongs, but filtering files by it is the actual use case.The age fields stay strings. The released corpus contains
2-4andunknownalongside decimals and distinguishes10.5from10.50, which is also why they do not go toSubject.age_at_sampling— a float in years could represent neither, and an annotation row names no donor to build aSubjectfrom in any case.Both
extra.encode.annotation_typeandextra.encode.organismgain an index and a place in the distinct-values allowlist. The ENCODE sync writesfilesdirectly and never reaches the materializer that owns the rest of its keys, so nothing else would create them, and the flagship filter would otherwise be a full scan of a ~300,000 document collection on an unauthenticated endpoint. The allowlist entry matters because filtering by annotation type is only useful if the vocabulary is discoverable.Behavior changes for existing clients
.bedpeandbigInteractfiles already in the experiment corpus changefile_format.namefromBED/bigBedtobedpe/bigInteract, so a saved query filteringfileFormat: {name: ["BED"]}stops matching them. Their/indexresponses become 404 and/dataserves the raw upstream file. This is the intended correction — the index they returned before was built from the first locus only. Tileset endpoints that understand paired intervals are planned separately.extra.encode.assemblyis populated for the first time. It has always been published in the SDL and never written, so a client filtering on it previously matched nothing at all.Test cases
tests/test_ontology_mappings.pytests/test_ontology_mappings.pytests/test_ontology_mappings.pytests/test_ontology_mappings.pybedpeandbigInteracttests/test_ontology_mappings.pytests/test_ontology_mappings.pyget_data_typeis calleddata_typecoveragetests/test_encode.pyENCODE_ANNOTATION_TYPESin the environmentannotation_types_from_envis calledtests/test_encode.pyannotation_types_from_envis calledtests/test_encode.pyannotation_types_from_envis calledtests/test_encode.pyannotation_types_from_envis calledtests/test_encode.pytests/test_encode.pytests/test_encode.pyannotation[<type>]tests/test_encode.pytests/test_encode.pytests/test_encode.pytransform_annotation_to_c2m2is calledtests/test_encode.pytransform_annotation_to_c2m2is calledannotation_typeis set on both the file and its datasettests/test_encode.pytransform_annotation_to_c2m2is calledtests/test_encode.pytransform_annotation_to_c2m2is calledtests/test_encode.pytransform_annotation_to_c2m2is calledpersistent_idpoints at/annotations/tests/test_encode.pytransform_annotation_to_c2m2is calledtests/test_encode.pytransform_annotation_to_c2m2is calledtests/test_encode.pyAssay term nameis emptytransform_annotation_to_c2m2is calledexperiment_typeand the file noassay_typetests/test_encode.pyFileMetadataModeltests/test_encode.pytransform_to_c2m2is calledpersistent_idpoints at/experiments/tests/test_encode.pytransform_to_c2m2is calledtests/test_encode.pytransform_to_c2m2is calledtests/test_sync.py_sync_encoderunstests/test_sync.py_sync_encoderunstests/test_sync.py_sync_encoderunstests/test_sync.py_sync_encoderunstests/test_sync.py_sync_encoderunstests/test_sync.py_sync_encoderunstests/test_sync.pyCancelledError_sync_encoderunstests/test_sync.py_sync_encoderunstests/test_sync.py_sync_encoderunstests/test_sync.py_sync_encoderunstests/test_sync.py_sync_encoderunstests/test_sync.py_sync_encoderunstests/test_sync.py_sync_encoderunstests/test_sync.pyENCODE_ANNOTATION_TYPESin the environment_sync_encoderunstests/test_inputs.pyto_dictthento_querytests/test_inputs.pyto_querybuilds the predicatetests/test_schema.pytests/test_schema.pytests/test_schema.pytests/test_schema.pytests/test_schema.pydistinctValuesis asked for the annotation typetests/test_index.pystream_index_fileis calledtests/test_data.pystream_fileis calledtests/test_indexes.py