airr-community / airr-standards

AIRR Community Data Standards

https://docs.airr-community.org

Creative Commons Attribution 4.0 International

35 stars 23 forks source link

Clone-schema-updates #778

Open scharch opened 3 months ago

scharch commented 3 months ago

Draft of updates to Clone. Closes:

201 (incidentally)

317

333

378

769

Open questions, both related to the switch from Repertoire to RepertoireGroup:

[x] Do we need a more specific way to address the _ids in members (etc)? Currently, one would have to search each Repertoire in the RepertoireGroup, which seems inefficient. OTOH, having members (etc) be arrays of arrays is uuuuuuugly (and not sure it's even allowed).
[x] What do we do about data_processing_id? Can RepertoireGroups get their own DataProcessing objects?

Other remaining tasks before this can be merged:

[ ] Sync schemas
[ ] Unit tests for loading Clones
[ ] Docs updates
[ ] ...?

scharch commented 3 months ago

From the call:

push members and inferred_members down to a single revived but skinny Node array
Node should have both sequence_id and cell_id with only one populated (but maybe add a level shortcut too?)
Change Cell.inferred and Rearrangement.inferred to an enum to match Repertoire.type and add this to Node as well
- Does an inferred Node need to be part of a Repertoire at all? Maybe this allows us to drop that bit entirely...
- preference was for 'observed' or 'physical' in the current enum
Add repertoire_id to Node to facilitate retrieval of underlying data
Did not discuss what to do about data_processing_id

javh commented 3 months ago

Random thoughts:

For repertoire_type what about observed instead of physical?
Maybe the inferred boolean for Rearrangement could instead be sequence_type and use the same enum as repertoire_type? Same for Cell and a cell_type field.
As an alternative to level/members in Clone, we could have sequences / cells which are explicitly typed and set the expectation to only populate one (we could still retain level in this case).
Another alternative to level/members in Clone would be to have a different version of Node that contains the sequence_id/cell_id fields (without the junction and whatnot), and possibly the level/sequence_type/cell_type, and possibly the repertoire_id so that doesn't need to be fetched out of RepertoireGroup somehow. Ie, members contains Node references.

schristley commented 3 months ago

[ ] What do we do about data_processing_id? Can RepertoireGroups get their own DataProcessing objects?

@scharch It's best to think of DataProcessing as an independent, orthogonal object to the other (top-level) AIRR objects. Anything that can be part of DataProcessing should have a data_processing_id so yes including one in RepertoireGroup makes sense.

scharch commented 1 month ago

OK, let me know how this looks.

I changed "physical" to "observed" and made rearrangement_type and cell_type enums instead of inferred booleans
I re-added Node as a way get repertoire_id in. It feels a little "overweight" but probably the best solution?