Support querying and displaying circuits by derivation type#642
Open
pgetta wants to merge 4 commits into
Open
Conversation
Expose circuit derivations on GET /circuit, derived from the existing
Derivation table (no migration, no denormalized column).
- Filter (always available), both directions mirroring the read fields:
generated_derivation__derivation_type (circuit is the generated/derived
entity) and used_derivation__derivation_type (circuit is the used/source
entity), each with an __in variant. No source-type restriction, so e.g.
an emodel->circuit emodel_circuit derivation matches on the generated side.
- Read columns (opt-in via ?expand=...): generated_derivations and
used_derivations, expandable independently. Unexpanded directions are
omitted; expanded but empty serialize as []. Load-aware properties keep
this safe under raiseload("*") via two viewonly relationships on Circuit.
The paginated id subquery in `_with_subquery` selected ids without DISTINCT, so a one-to-many filter join (e.g. the circuit derivation-type filters, where a circuit can match several Derivation rows) duplicated the row. OFFSET/LIMIT then paged over the duplicated rows while total_items used count(distinct id), so a matching entity could repeat across pages or be skipped. Select DISTINCT ids in the subquery so the limit window operates on distinct entities. This also hardens the pre-existing contribution/mtype/used/generated one-to-many filters. DISTINCT is valid here because every ORDER BY element is already part of the subquery select list. Add a regression test asserting a circuit matched by multiple derivation rows is counted once and does not reappear on a second page.
Codecov Report✅ All modified and coverable lines are covered by tests.
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
| query = query.options( | ||
| selectinload(Circuit.derivations_as_used).joinedload(Derivation.generated) | ||
| ) | ||
| return query |
Collaborator
There was a problem hiding this comment.
For discussion: it would be nice if the possibility to filter by derivation type and get the list of derivations is made available to any entity.
However, it would require changes:
- to the _load function in every entity (change to many files, with many repetitions, so I would avoid that)
- or some changes in queries/common.py for read_many/read_one
Thoughts? Could it be useful already for other derivation types?
If the change adds too much complexity, the generalization can be postponed.
| def read_one( | ||
| user_context: UserContextDep, | ||
| db: SessionDep, | ||
| id_: uuid.UUID, |
Collaborator
There was a problem hiding this comment.
Can it be useful to add the expand parameter to read_one as well, not only read_many?
For cell morphologies, it's present in read_one and not in read_many (for performance reasons), but it would be cleaner to be consistent across endpoints.
Contributor
Author
There was a problem hiding this comment.
Good idea, I've extended also the read_one
Wire the `expand` query param into read_one and admin_read_one, mirroring read_many/admin_read_many: switch the response schema to CircuitExpandedRead when expand is set and pass partial(_load, expand=expand). Add a test exercising expand on both the user and admin get-one routes.
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
Implements the backend for openbraininstitute/prod-explore-functionality#517 — the Circuits overview needs a "Derivation type" column and a filter by derivation type, general enough to absorb new derivation types.
This is derived entirely from the existing normalized
Derivationtable (used→generated, indexedderivation_type).Filter — narrow the circuit list by derivation type
Both directions are exposed, mirroring the
Derivationmodel's roles:?generated_derivation__derivation_type=circuit_extraction— circuits that were derived this way ("how it was derived")?used_derivation__derivation_type=circuit_extraction— circuits that are a source of such a derivation ("what was derived from it")__invariants supported for both.No source-type restriction: a circuit derived from a non-circuit source (e.g.
emodel_circuit) surfaces that type too. Implemented with the establishedquery_params_factory+filter_joinsjoin pattern (enum, so it's a plain filter, not a facet).Read — opt-in derivation lists via
expandDerivation data loads only when requested, per direction, via the
expandquery param (same pattern ascell_morphology/em_cell_mesh):?expand=generated_derivations→ each entry carries the sourceusedentity?expand=used_derivations→ each entry carries the resultinggeneratedentity?expand=generated_derivations&expand=used_derivations)Backed by two view-only relationships on
Circuitplus load-aware properties: an un-expanded direction serializes asnull(no extra query, never tripsraiseload); an expanded-but-empty direction serializes as[]. Nothing loads on the defaultGET /circuit.Pagination fix (one-to-many filter joins)
The derivation filter join is one-to-many (a circuit can match several
Derivationrows). The shared paginated id subquery (_with_subquery) selected ids withoutDISTINCT, soOFFSET/LIMITpaged over duplicated rows whiletotal_itemsusedcount(distinct id)— a matching circuit could repeat across pages or be skipped. Fixed by selectingDISTINCTids in the subquery, which also hardens the pre-existingcontribution/mtype/used/generatedone-to-many filters.DISTINCTis valid because everyORDER BYelement is already in the subquery select list.