Repository navigation
organize: name files with BIDS labels (+), refuse indistinguishable identifiers - #1981
Draft
yarikoptic-gitmate wants to merge 1 commit into
Draft
yarikoptic-gitmate wants to merge 1 commit into
yarikoptic-gitmate wants to merge 1 commit into
Conversation
BIDS 1.11.0 allows "+" in labels (bids-standard/bids-specification#1926). organize used to replace only some characters (_*\/<>:|"'?%@;, whitespace and .) with "-", which BIDS does not allow in labels and which makes `sub-fly-226` ambiguous to parse. Now each character other than an ASCII letter, an ASCII digit or "+" becomes "+", one character at a time, so e.g. subject_id "fly_226" gives `sub-fly+226`. This is the same as `dandischema.utils.sanitize_label` (dandi/dandi-schema, branch bids-label-sanitization), and a test checks the two agree when dandischema provides it. Since distinct identifiers can now end up with the same label, organize fails, listing them, if distinct subject IDs (or distinct session, tissue sample, slice or cell IDs of a subject) would get the same label, or labels differing only in case, which BIDS forbids and which collide on case-insensitive file systems. Among the 1,001 public draft Dandisets on DANDI, organizing from scratch would now fail for 5: 000246 ("AAV V15 D5" vs "AAV V15-D5") and 001250 ("vivo-marques-smith" vs "vivo_marques_smith"), which already got merged into one folder; 001702 ("FAD+_..." vs "FAD-_..."), which would be merged now; and 001428 and 001791, whose subject IDs differ only in case. Files organized by older versions keep their names: `get_existing_labels()` collects the labels in the names of the `sub-*/*.nwb` files already in the Dandiset, and a value whose older label is among them keeps it. Without that, files of 365 of those 1,001 Dandisets would be renamed when reorganized in place, and new files would go to e.g. `sub-fly+226/` next to the existing `sub-fly-226/`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PvaGRZLHuwDgtWrKMFiMZ8
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1981 +/- ##
==========================================
+ Coverage 91.18% 91.26% +0.08%
==========================================
Files 95 95
Lines 14740 14861 +121
==========================================
+ Hits 13440 13563 +123
+ Misses 1300 1298 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
|
(a) I don't think this should be in DANDI CLI (b) This is the kind of thing that (c) BIDS still isn't supporting for the majority of our neurophysiology modalities (d) How does this interact with the ad-hoc DANDI validation, and should we not emphasize the (+) convention there first? (e) What issue number is this closing? |
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
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
+in labels (bids-standard/bids-specification#1926).dandi organizenow builds labels by replacing every character other than an ASCII letter, an ASCII digit or+with+, one character at a time. For example,subject_idfly_226becomessub-fly+226; it used to besub-fly-226, which is not a valid BIDS label. The function is the same asdandischema.utils.sanitize_label()from dandi/dandi-schema#447, and a test checks that they agree whenever dandischema provides it.organizefails with a list of the offending IDs in two cases:get_existing_labels()collects the labels in the names of thesub-*/*.nwbfiles already in the Dandiset. A value whose older label is among them keeps that label. That covers the-replacement, and also a uniquesanitize_label()match for names written by versions that kept,or spaces. Reorganizing in place renames nothing, and new files of an existing subject or session go next to its existing files.docs/source/cmdline/organize.rstand theorganizehelp text describe all three.Impact on DANDI
I computed this from the
dandisets/<id>/draft/assets.jsonldmanifests of the 1,001 public draft Dandisets (606,651 assets, 2026-11-05):Renames avoided. 226,518 assets in 365 Dandisets have labels that the new rule would change, so reorganizing them in place would rename them without the reuse of existing names. The characters responsible are mostly
-(380,701 occurrences), then#, space,(,),,and&.Dandisets that would now fail to organize from scratch:
AAV V15 D5,AAV V15-D5vivo-marques-smith,vivo_marques_smithFAD+_L_CA2 20X_SF2,FAD-_L_CA2 20X_SF2, …Aged mice,aged miceBarrel500 2,barrel500 2Reorganizing 001702 in place still works, because its existing distinct names are reused. No session IDs collide within a subject.
No identifier contains non-ASCII characters, and none sanitizes to an empty label.
Notes and open questions
+. Collapsing would merge more distinct IDs, e.g.…_-0.4V vs Eref…and…_0.4V vs Eref…in 000983, and4484and4484?in 001195.+as list separator. dandi-cli already joins lists with+, as in_probe-1+2and the modalities suffix_behavior+ecephys.probe-A+Bcan therefore mean two probes, or one probe namedA_B. Either is a single valid BIDS label, but a reader can't tell which.+in URLs. In URL query strings+decodes to a space, so a path in?path=…must be sent as%2B.requestsdoes that; hand-written URLs may not.validate_organized_path()(LABELREGEX) is unchanged, so names from older versions, including those with,or spaces, still validate. A HINT-level "not a BIDS label" result could follow.minor.Testing
pytest dandi/tests/test_organize.py: 68 passed. One test is skipped with dandischema 0.14.0, which lackssanitize_label; with the Add BIDS sanitize_label(); use it to match file-name labels to metadata identifiers dandi-schema#447 branch installed, that test passes too.DANDI_TESTS_NONETWORK=1and-m "not integration": 941 passed, 11 failed, 402 skipped. The same 11 fail onmasterhere: 10 needbids-validator-deno, which isn't installed, andtest_helptextneedsdandionPATH. Network and Docker-based integration tests were not run; CI covers them.Generated by Claude Code