Skip to content

[ENH] BEP032: Require the sample entity for icephys and enforce samples.tsv - #2480

Open
bendichter wants to merge 1 commit into
bids-standard:bep032-reviewfrom
bendichter:bep032-sample-entity
Open

[ENH] BEP032: Require the sample entity for icephys and enforce samples.tsv#2480
bendichter wants to merge 1 commit into
bids-standard:bep032-reviewfrom
bendichter:bep032-sample-entity

Conversation

@bendichter

@bendichter bendichter commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Implements the decisions we reached in the working group meeting on 2026-07-29 about the sample-<label> entity, addressing #2307 (comment). This targets the bep032-review branch, so it merges into #2307 rather than into master.

Requirement Level

The entity is now REQUIRED for icephys and remains OPTIONAL for ecephys. We agreed that an intracellular recording is always made from a discrete sample, so the requirement holds even when a subject yields only one sample, and even when the recording is made in vivo, since the patched cell is itself the sample. Requiring it in the single-sample case also avoids having to rename files when a second sample is added later. This mirrors how microscopy treats the entity.

In the schema this meant splitting the microephys file rule, since it previously covered both datatypes in one block with sample: optional. The icephys rule now inherits from the ecephys rule and overrides the datatype, suffix, and requirement level, following the pattern already used for photo__micr.

Documentation

The entity was previously introduced only in the Photos section, framed as being for histological photos, even though the intracellular examples use it in every filename. It now has a dedicated section near the top of the page covering the requirement levels, the granularity convention, and the relationship to samples.tsv. The mention in the Photos section is now a back-reference to that section.

On granularity, the text states that a sample is the finest unit of biological material from which signals are measured: the individual cell for patch clamp, and typically the slice or cell population for in vitro extracellular recordings. Where several samples share a source, for example several cells patched from one slice, the source may also be described in samples.tsv and linked with derived_from. Such a parent sample generally does not appear as an entity in recording filenames, since it is not itself what was measured.

Enforcing samples.tsv

The specification already states, in the description of the samples file, that samples.tsv is REQUIRED if sample-<label> appears in any filename in the dataset. The only check enforcing that was keyed on "micr" in dataset.modalities, so it never fired for microephys data. This adds a check keyed on the presence of the entity instead, which applies to both icephys and ecephys. The existing microscopy check is left alone so that its dataset-level reporting is unchanged.

Verification

bidsschematools tests pass (81 passed, 1 skipped), as do pre-commit on the changed files, npm run remark, and the Latin-phrase check. I confirmed the rendered filename templates show _sample-<label> as required for icephys and optional for ecephys, and that every icephys file in the existing intracellular example already carries the entity, so that example remains valid.

@lzehl, this is the first of the items from the meeting. Happy to adjust the wording of the new section if it does not match what we discussed.

Introduce the sample-<label> entity in a dedicated section of the
microelectrode electrophysiology page rather than defining it in passing in
the Photos section, and record the requirement levels agreed by the working
group: REQUIRED for icephys, including the single-sample and in vivo cases,
and OPTIONAL for ecephys.

Split the microephys file rule so that icephys requires the entity while
ecephys keeps it optional.

Add a check requiring samples.tsv whenever the sample entity is present in a
filename. The specification already states this requirement in the
description of the samples file, but the only check enforcing it was keyed on
the micr modality, so it never fired for microephys data.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@bendichter
bendichter requested a review from erdalkaraca as a code owner July 29, 2026 16:11
@codecov

codecov Bot commented Jul 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.09%. Comparing base (3f661db) to head (4d25a0a).

Additional details and impacted files
@@              Coverage Diff               @@
##           bep032-review    #2480   +/-   ##
==============================================
  Coverage          83.09%   83.09%           
==============================================
  Files                 22       22           
  Lines               1698     1698           
==============================================
  Hits                1411     1411           
  Misses               287      287           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@SylvainTakerkart

Copy link
Copy Markdown

Hi @bendichter , I can review this PR if that's ok with you...

@bendichter

Copy link
Copy Markdown
Contributor Author

@SylvainTakerkart that would be great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants