While adding numberOfSessions in #409, I noticed the aggregator has the same latent issue for samples (and probably cells/slices) that the session work is fixing:
stats["tissuesample"], stats["cell"], and stats["slice"] are flat lists of identifier strings, deduplicated globally rather than per-subject. So if sub-01 and sub-02 both have a sample-1 (or both reference a BioSample with the same identifier), the aggregator collapses them into a single entry and numberOfSamples is off by one.
Relevant code:
- Filename-derived samples:
dandischema/metadata.py:537 (the ("sample", "sample-", "tissuesample") entry in the entity loop) appends to stats["tissuesample"] keyed only on the sample id.
wasDerivedFrom / BioSample-derived samples: dandischema/metadata.py:471 (_get_samples) and :522-527. Identifiers go through sanitize_value(value["identifier"]) and then dedupe globally per sampleType (cell, slice, tissuesample).
- Final tally:
dandischema/metadata.py:589 (numberOfSamples = len(tissuesample) + len(slice)) and :591 (numberOfCells = len(cell)).
The cell/slice path is less likely to bite in practice because BioSample identifiers are typically globally unique, but the filename sample-* path is much more likely to collide across subjects.
Proposed fix: key on (subject, sample) / (subject, cell) / (subject, slice) pairs the same way #409 keys on (subject, session), then take len(unique_pairs).
Caveats:
- This changes the value of existing fields (
numberOfSamples, numberOfCells) for any dandiset where the same id appears under multiple subjects. That's a semantic shift, so it should land deliberately and probably be called out in the changelog.
- For the
_get_samples path, the BioSample identifier is supposed to be globally unique. If we move to (subject, sample-id) keying there, the subject context needs to be plumbed through _get_samples.
Spun out of #409 review discussion.
While adding
numberOfSessionsin #409, I noticed the aggregator has the same latent issue for samples (and probably cells/slices) that the session work is fixing:stats["tissuesample"],stats["cell"], andstats["slice"]are flat lists of identifier strings, deduplicated globally rather than per-subject. So ifsub-01andsub-02both have asample-1(or both reference aBioSamplewith the sameidentifier), the aggregator collapses them into a single entry andnumberOfSamplesis off by one.Relevant code:
dandischema/metadata.py:537(the("sample", "sample-", "tissuesample")entry in the entity loop) appends tostats["tissuesample"]keyed only on the sample id.wasDerivedFrom/BioSample-derived samples:dandischema/metadata.py:471(_get_samples) and:522-527. Identifiers go throughsanitize_value(value["identifier"])and then dedupe globally persampleType(cell,slice,tissuesample).dandischema/metadata.py:589(numberOfSamples = len(tissuesample) + len(slice)) and:591(numberOfCells = len(cell)).The cell/slice path is less likely to bite in practice because BioSample identifiers are typically globally unique, but the filename
sample-*path is much more likely to collide across subjects.Proposed fix: key on
(subject, sample)/(subject, cell)/(subject, slice)pairs the same way #409 keys on(subject, session), then takelen(unique_pairs).Caveats:
numberOfSamples,numberOfCells) for any dandiset where the same id appears under multiple subjects. That's a semantic shift, so it should land deliberately and probably be called out in the changelog._get_samplespath, the BioSampleidentifieris supposed to be globally unique. If we move to(subject, sample-id)keying there, the subject context needs to be plumbed through_get_samples.Spun out of #409 review discussion.