feat: add numberOfSessions to AssetsSummary#409
Conversation
Aggregator counts unique (subject, session) pairs parsed from BIDS-style sub-* and ses-* tokens in asset paths (filename first, then directory components when the session token is omitted from the filename). Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #409 +/- ##
==========================================
+ Coverage 48.31% 48.47% +0.16%
==========================================
Files 19 19
Lines 2434 2463 +29
==========================================
+ Hits 1176 1194 +18
- Misses 1258 1269 +11
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
# Conflicts: # dandischema/metadata.py
| if not found.get("session") and part.startswith("ses-"): | ||
| found["session"] = part.split("ses-", 1)[1] | ||
| # Fallback: ses- tokens that appear only in directory components (e.g. | ||
| # `sub-X/ses-Y/foo_acq-Z_bold.nii.gz`) should still be counted. To form | ||
| # the (subject, session) pair we also accept a directory-only subject, | ||
| # but we do not add such a subject to stats["subjects"] — subject | ||
| # counting remains driven by the filename and wasAttributedTo, matching | ||
| # prior behavior. | ||
| if not found.get("session"): | ||
| dir_subject = found.get("subject") | ||
| dir_session: Optional[str] = None | ||
| for directory in asset_path.parts[:-1]: | ||
| for part in directory.split("_"): | ||
| if dir_subject is None and part.startswith("sub-"): | ||
| dir_subject = part.split("sub-", 1)[1] | ||
| if dir_session is None and part.startswith("ses-"): | ||
| dir_session = part.split("ses-", 1)[1] | ||
| if dir_subject is not None and dir_session is not None: | ||
| found.setdefault("subject", dir_subject) | ||
| found["session"] = dir_session | ||
| stats["sessions"] = stats.get("sessions", []) | ||
| if found.get("subject") and found.get("session"): | ||
| pair = (found["subject"], found["session"]) | ||
| if pair not in stats["sessions"]: | ||
| stats["sessions"].append(pair) |
There was a problem hiding this comment.
too aisloppy -- easier to HI compose the logic but first potentially even generalize 2 prior blocks into a 'for' loop, then if no session was found in filename, go through path's parts and split on ses- and be done if any matches.
There was a problem hiding this comment.
Done in cdb37ee — collapsed the sub-/sample-/ses- filename parsing into a single loop over an entities list, and simplified the session fallback to scanning asset_path.parts[:-1] for a ses-X directory and stopping at the first match. Dropped the directory-only subject fallback since BIDS files always carry sub- in the filename.
yarikoptic
left a comment
yarikoptic
left a comment
There was a problem hiding this comment.
THANK YOU for the PR. Needs a little a human touch ;)
yarikoptic
added
enhancement
Per review feedback: generalize the per-prefix blocks (sub-, sample-, ses-) into a single loop over an entity list, and simplify the session fallback to a direct scan of path parts. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
|
Is the |
|
yes, I think patch makes sense for this while major is 0 |
candleindark
left a comment
candleindark
left a comment
There was a problem hiding this comment.
LGTM as the targeted datasets are valid BIDS datasets. (See other comments for more details).
| numberOfCells: Optional[int] = Field(None, json_schema_extra={"readOnly": True}) | ||
| numberOfSessions: Optional[int] = Field( | ||
| None, json_schema_extra={"readOnly": True} | ||
| ) # BIDS ses-* tokens, counted as unique (subject, session) pairs |
There was a problem hiding this comment.
Possibly add to the comment that this count assumes the dataset is valid BIDS dataset. (For example, a session without subject will not be counted)
3 participants
Summary
numberOfSessions: intfield toAssetsSummary(subject, session)pairs parsed from BIDS-stylesub-*/ses-*tokens in asset paths — keying on the pair avoids the undercount when two subjects share a session id likeses-1sub-/sample-handling); directory components are scanned as a fallback soses-*tokens that appear only in the path (e.g.sub-X/ses-Y/foo_acq-Z_bold.nii.gz) are still countedCaveats
session_idfrom NWB asset metadata would need a separate signal that isn't currently extracted.Test plan
test_aggregate_number_of_sessionscovers: same subject across sessions, two subjects sharing a session id,ses-only in directory, and the no-ses-casetest_aggregatefixtures updated with expectednumberOfSessionsvaluestest_aggregation_bidsnow assertsnumberOfSessions == 2241 passed, 17 skipped🤖 Generated with Claude Code