Add BIDS sanitize_label(); use it to match file-name labels to metadata identifiers

dandischema/metadata.py

@@ -23,6 +23,7 @@
     _ensure_newline,
     dandi_jsonschema_validator,
     json_object_adapter,
+    sanitize_label,
     sanitize_value,
     strip_top_level_optional,
     validate_json,
@@ -511,18 +512,23 @@ def _add_asset_to_stats(assetmeta: Dict[str, Any], stats: _stats_type) -> None:
     # once in some incorrectly named datasets
     found: Dict[str, str] = {}
     asset_path = Path(assetmeta["path"])
-    # (entity key, BIDS prefix, unique-values bucket in stats; None if not aggregated)
+    # (entity key, BIDS prefix, unique-values bucket in stats; None if not aggregated).
+    # Values from file names go to separate "filename_*" buckets, since they can be
+    # differently sanitized versions of the identifiers in the metadata; see
+    # `_count_distinct`.
     entities = [
-        ("subject", "sub-", "subjects"),
-        ("sample", "sample-", "tissuesample"),
+        ("subject", "sub-", "filename_subjects"),
+        ("sample", "sample-", "filename_tissuesample"),
         ("session", "ses-", None),
     ]
     for part in asset_path.name.split(".")[0].split("_"):
         for key, prefix, bucket in entities:
             if not found.get(key) and part.startswith(prefix):
                 found[key] = value = part.split(prefix, 1)[1]
-                if bucket is not None and value not in stats[bucket]:
-                    stats[bucket].append(value)
+                if bucket is not None:
+                    stats[bucket] = stats.get(bucket, [])
+                    if value not in stats[bucket]:
+                        stats[bucket].append(value)
     # If ses- is absent from the filename, fall back to scanning the path
     # parts (BIDS keeps `ses-X` as its own directory).  Deliberately no such
     # fallback for sub-, so that subject counting behavior stays unchanged.
@@ -552,6 +558,25 @@ def add_if_missing(standard: dict) -> None:
         add_if_missing(models.ome_ngff_standard)
 
 
+def _count_distinct(stats: _stats_type, bucket: str) -> int:
+    """Pop `bucket` and ``filename_<bucket>`` from `stats` and count distinct values
+
+    ``stats[bucket]`` holds the distinct identifiers from the metadata (as
+    sanitized by `sanitize_value`) and ``stats["filename_" + bucket]`` the distinct
+    labels found in file names.  A label from a file name is counted only if it is
+    not merely a differently sanitized form of an identifier from the metadata,
+    i.e., if its `sanitize_label` form differs from theirs.  Otherwise values are
+    compared as they are, so e.g. ``5XFAD+_M1`` and ``5XFAD-_M1`` stay distinct
+    although their `sanitize_label` forms coincide.
+    """
+    from_metadata = cast(list, stats.pop(bucket, []))
+    from_filenames = cast(list, stats.pop(f"filename_{bucket}", []))
+    known = {sanitize_label(v) for v in from_metadata}
+    return len(from_metadata) + sum(
+        sanitize_label(v) not in known for v in from_filenames
+    )
+
+
 # TODO?: move/bind such helpers as .from_metadata or alike within
 #        model classes themselves to centralize access to those constructors.
 def aggregate_assets_summary(metadata: Iterable[Dict[str, Any]]) -> dict:
@@ -561,13 +586,13 @@ def aggregate_assets_summary(metadata: Iterable[Dict[str, Any]]) -> dict:
         _add_asset_to_stats(meta, stats)
     stats["numberOfBytes"] = stats.get("numberOfBytes", 0)
     stats["numberOfFiles"] = stats.get("numberOfFiles", 0)
-    stats["numberOfSubjects"] = len(stats.pop("subjects", [])) or None
+    stats["numberOfSubjects"] = _count_distinct(stats, "subjects") or None
     if stats["numberOfSubjects"]:
         # Must not happen. If does -- a bug in software
         assert stats["numberOfFiles"]
         assert stats["numberOfSubjects"] <= stats["numberOfFiles"]
     stats["numberOfSamples"] = (
-        len(stats.pop("tissuesample", [])) + len(stats.pop("slice", []))
+        _count_distinct(stats, "tissuesample") + len(stats.pop("slice", []))
     ) or None
     stats["numberOfCells"] = len(stats.pop("cell", [])) or None
     stats["numberOfSessions"] = len(stats.pop("sessions", [])) or None

dandischema/tests/test_metadata.py

@@ -776,6 +776,60 @@ def test_aggregate_number_of_sessions() -> None:
     assert "numberOfSessions" not in summary
 
 
+def _nwb_asset(path: str, *subject_ids: str) -> Dict[str, Any]:
+    asset = _bids_asset(path)
+    asset["wasAttributedTo"] = [
+        {"schemaKey": "Participant", "identifier": i} for i in subject_ids
+    ]
+    return asset
+
+
+@pytest.mark.ai_generated
+def test_aggregate_subjects_sanitized_names() -> None:
+    # File name produced by an older dandi-cli, which kept "," (as in
+    # https://dandiarchive.org/dandiset/000550): the same subject, counted once
+    data = [
+        _nwb_asset(
+            "sub-Fig6-5xBi-0,83MHz-08/sub-Fig6-5xBi-0,83MHz-08_image.nwb",
+            "Fig6_5xBi_0,83MHz_08",
+        )
+    ]
+    assert aggregate_assets_summary(data)["numberOfSubjects"] == 1
+
+    # Legacy ("-") and BIDS ("+") sanitized names of the same subject
+    data = [
+        _nwb_asset("sub-fly-226/sub-fly-226_ses-1_ecephys.nwb", "fly_226"),
+        _nwb_asset("sub-fly+226/sub-fly+226_ses-2_ecephys.nwb", "fly_226"),
+    ]
+    assert aggregate_assets_summary(data)["numberOfSubjects"] == 1
+
+    # Distinct subjects stay distinct even if their sanitized forms coincide
+    # (as in https://dandiarchive.org/dandiset/001702), with or without metadata
+    data = [
+        _nwb_asset("sub-5XFAD+-M1/sub-5XFAD+-M1_ses-1.nwb", "5XFAD+_M1"),
+        _nwb_asset("sub-5XFAD--M1/sub-5XFAD--M1_ses-1.nwb", "5XFAD-_M1"),
+    ]
+    summary = aggregate_assets_summary(data)
+    assert summary["numberOfSubjects"] == 2
+    assert summary["numberOfSessions"] == 2
+    for asset in data:
+        del asset["wasAttributedTo"]
+    summary = aggregate_assets_summary(data)
+    assert summary["numberOfSubjects"] == 2
+    assert summary["numberOfSessions"] == 2
+
+    # A file name which does not correspond to the subject metadata still adds
+    # a subject (as in https://dandiarchive.org/dandiset/001869)
+    data = [
+        _nwb_asset("sub-fly-226/sub-fly-226_ses-1.nwb", "fly_0"),
+        _nwb_asset("sub-fly-226/sub-fly-226_ses-2.nwb", "fly_0"),
+        _nwb_asset("sub-fly-228/sub-fly-228_ses-1.nwb", "fly_228"),
+    ]
+    assert aggregate_assets_summary(data)["numberOfSubjects"] == 3
+    with pytest.raises(AssertionError):
+        aggregate_assets_summary(data[:1])
+
+
 class TestGetJsonschemaValidator:
     @pytest.mark.parametrize(
         "schema_version, schema_key, expected_error_msg",

dandischema/tests/test_utils.py

@@ -1,4 +1,5 @@
 from contextlib import nullcontext
+import re
 from typing import Any, Dict, List, Optional, Union, cast
 from unittest.mock import patch
 
@@ -9,10 +10,12 @@
 
 from dandischema.exceptions import JsonschemaValidationError
 from dandischema.utils import (
+    BIDS_LABEL_REGEX,
     _ensure_newline,
     dandi_jsonschema_validator,
     jsonschema_validator,
     name2title,
+    sanitize_label,
     sanitize_value,
     strip_top_level_optional,
     validate_json,
@@ -99,6 +102,39 @@ def test_sanitize_value() -> None:
     assert sanitize_value("A\"'B") == "A--B"
 
 
+@pytest.mark.ai_generated
+@pytest.mark.parametrize(
+    "value,label",
+    [
+        ("fly226", "fly226"),
+        ("fly_226", "fly+226"),
+        ("fly-226", "fly+226"),
+        ("a+b", "a+b"),
+        # identifiers from https://dandiarchive.org/dandiset/000550 and 000703
+        ("Fig6_5xBi_0,83MHz_08", "Fig6+5xBi+0+83MHz+08"),
+        ("nac (2)", "nac++2+"),
+        # one-to-one replacement: nothing is collapsed or stripped
+        ("-0.4V", "+0+4V"),
+        ("0.4V", "0+4V"),
+        ("A B\t C", "A+B++C"),
+        ("µm", "+m"),
+    ],
+)
+def test_sanitize_label(value: str, label: str) -> None:
+    assert sanitize_label(value) == label
+    assert re.fullmatch(BIDS_LABEL_REGEX, label)
+
+
+@pytest.mark.ai_generated
+@pytest.mark.parametrize("field", ["non-extension", "extension"])
+def test_sanitize_label_of_sanitized_value(field: str) -> None:
+    # Labels in file names produced with sanitize_value map to the same BIDS label
+    # as the identifiers they were produced from
+    value = "".join(chr(i) for i in range(32, 127)) + " \t\nµé"
+    for v in [value, *value]:
+        assert sanitize_label(sanitize_value(v, field)) == sanitize_label(v)
+
+
 @pytest.fixture
 def draft7_schema() -> dict:
     """

dandischema/utils.py

@@ -134,6 +134,8 @@ def sanitize_value(value: str, field: str = "non-extension", sub: str = "-") ->
 
     Based on dandi.organize._sanitize_value.
 
+    See `sanitize_label` for producing labels that are valid in BIDS.
+
     .. versionchanged:: 0.8.3
 
             ``sanitize_value`` added
@@ -144,6 +146,39 @@ def sanitize_value(value: str, field: str = "non-extension", sub: str = "-") ->
     return value
 
 
+#: Regular expression for a BIDS label.  BIDS 1.11.0 allows ``+`` in labels
+#: (https://github.com/bids-standard/bids-specification/pull/1926).
+BIDS_LABEL_REGEX = r"[0-9a-zA-Z+]+"
+
+
+def sanitize_label(value: str) -> str:
+    """Make `value` (e.g., a subject or session identifier) a BIDS label
+
+    Every character other than an ASCII letter, an ASCII digit or ``+`` is
+    replaced with ``+``, so the result matches `BIDS_LABEL_REGEX` unless
+    `value` is empty.  The replacement is one-to-one: runs are not collapsed
+    and nothing is stripped, so identifiers such as ``"-0.4V"`` and ``"0.4V"``
+    stay distinct.
+
+    Since `sanitize_value` only replaces characters which this function
+    replaces too, ``sanitize_label(sanitize_value(v)) == sanitize_label(v)``
+    for any `v`.  This makes `sanitize_label` suitable for matching a label in
+    a file name produced with `sanitize_value` (as by older dandi-cli
+    versions) against the identifier it was produced from.
+
+    Parameters
+    ----------
+    value : str
+        Value to sanitize.
+
+    Returns
+    -------
+    str
+        The sanitized value.
+    """
+    return re.sub(r"[^0-9a-zA-Z+]", "+", value)
+
+
 def dandi_jsonschema_validator(schema: dict[str, Any]) -> JsonschemaValidator:
     """
     Create a JSON Schema validator appropriate for validating instances against the