[ENH]: BEP032 formal review - #2307
Conversation
* Add first BEP032 objects * Add ephys modality, start adding _channels but ran into a question on _acq- * Add _acq- to ephys, add _channels and _coords * Draft for probes TSV file * Fix preferably typo * Comment out not yet defined iephys and add ephys to datatypes where it was missing * Added probes file * Describe modality/datatype as "Microelectrode Electrophysiology" otherwise "Electrophysiology" is too broad. Thanks @dorahermes for the note * Remove iephys entirely since I think it was just inspired by DANDI layout not BEP032 * Initial content for ephys document and initial filename template tree * pacify pre-commit * add to TOC * semantic line break and fix * Addressed minor left over lint complains * Add "Microelectrode" specialization for Electrophysiology in the context menu * Fix up for bad latin and adjust wording a bit * Replace fancy unicode “ with simple " . I think this is what might have freaked out `schemacode_ci / windows-latest with Python 3 (pull_request) ` (windows only!) fails with a bunch of ``` 2024-04-19T19:13:44.2815607Z self = <encodings.cp1252.IncrementalDecoder object at 0x0000028D6A14D850> 2024-04-19T19:13:44.2818292Z input = b'---\nHED:\n name: HED\n display_name: HED Tag\n description: |\n Hierarchical Event Descriptor (HED) Tag.\n ...ed or ideal position along the z axis.\n anyOf:\n - type: number\n - type: string\n enum:\n - n/a\n' 2024-04-19T19:13:44.2820386Z final = True 2024-04-19T19:13:44.2820609Z 2024-04-19T19:13:44.2820821Z def decode(self, input, final=False): 2024-04-19T19:13:44.2821664Z > return codecs.charmap_decode(input,self.errors,decoding_table)[0] 2024-04-19T19:13:44.2823260Z E UnicodeDecodeError: 'charmap' codec can't decode byte 0x9d in position 2105: character maps to <undefined> 2024-04-19T19:13:44.2824229Z 2024-04-19T19:13:44.2824768Z C:\hostedtoolcache\windows\Python\3.12.3\x64\Lib\encodings\cp1252.py:23: UnicodeDecodeError ``` * Use redirected to www.datalad.org and www.nwb.org website URLs and few others * BEP032: ephys -> microephys with {icephys, ecephys} modalities/suffixes (#1806) * RF: to have "microephys" (Microelectrode physiology) for modality and icephys and ecephys for suffixes and datatypes * Reflecting decision of having two separate datatypes under the Microelectrode Electrophysiology #1800 (comment) Consensus reached during working group meeting on 2024-05-15: - modality = "Microelectrode Electrophysiology" - datatypes = "icephys" and "ecephys" - suffixes = "_icephys" and "_ecephys" * Adjust wording to Horea's recommendation * Various fixups and tune ups to wording from code review * Clarify wording and do mention `microephys` as shorthand for Microelectrode Electrophysiology * Adding markdown portion of the Bep032 (#1960) * test * MACROS___make_suffix_table * markdown * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * undoing changes to yaml files * adding chanell * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Update src/modality-specific-files/microelectrode-electrophysiology.md Co-authored-by: Yaroslav Halchenko <debian@onerussian.com> * Update src/modality-specific-files/microelectrode-electrophysiology.md Co-authored-by: Yaroslav Halchenko <debian@onerussian.com> * Update src/modality-specific-files/microelectrode-electrophysiology.md Co-authored-by: Yaroslav Halchenko <debian@onerussian.com> * Update src/modality-specific-files/microelectrode-electrophysiology.md Co-authored-by: Yaroslav Halchenko <debian@onerussian.com> * Update src/modality-specific-files/microelectrode-electrophysiology.md Co-authored-by: Yaroslav Halchenko <debian@onerussian.com> * Update src/modality-specific-files/microelectrode-electrophysiology.md Co-authored-by: Yaroslav Halchenko <debian@onerussian.com> * Update src/modality-specific-files/microelectrode-electrophysiology.md Co-authored-by: Yaroslav Halchenko <debian@onerussian.com> * adding General ephys metadata * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Examples of real datasets * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Minor tuneups to formatting --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Yaroslav Halchenko <debian@onerussian.com> * [ENH] Schema changes related to BEP032 (#1826) * [FIX] Minor YAML formatting * [ENH] Added additional birthdate column for participants.tsv * [ENH] Added age_category, age_type and corresponding enums * [ENH] Added surgery_date column * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Reverted changes that better suit in a PR against master * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * [ENH] Added tabular_data file and columns for _probes.tsv * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * [ENH] Added tabular_data rules and columns for _electrodes.tsv * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * [ENH] Added tabular_data rules for _electrodes.tsv * [ENH] Added tabular_data rules and columns for _channels.tsv * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * [FIX] Fixed pre-commit errors * [ENH] Added additional ephys columns for events.tsv * [ENH] Added tabular data rules for events.tsv to include ephys related columns * [ENH] Added metadata entities for ephys setup metadata * [ENH] Created sidecar rule file for ephys metadata and added data origin and setup fields * Made schema changes to match microephys data type * Renamed rule files from ephys to microephys * Made schema changes to match microephys data type * [ENH] Added objects and rules for coordinate system sidecar file * [ENH] Added processing microephys metadata field in rules * [ENH] Added pharmaceuticals microephys metadata field in rules * [ENH] Added supplementary microephys metadata field and rules * [ENH] Added sample microephys metadata rules * [ENH] Added task microephys metadata rules * Fixed example language convention * Added microephys in schema rule modality * Corrected SampleThickness to SliceThickness * Add yaml document separator for microephys rules file It is optional, and adding does not solve anything but makes it consistent with the other files in the folder * BF: should be a dict, not a list of dicts --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Yaroslav Halchenko <debian@onerussian.com> * Some fixes of formatting and URLs * Adjusted some formatting and some wording at the beginning of the section * Few fixes to the schema from review Co-authored-by: Peyman Najafi <najafe.peyman@gmail.com> * Adopting the markdown line lengths for BEP032 (#2012) * cutting line lenghts * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * marking json examples * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * microephysChannels --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * Fix tabulation in a .tsv + convert to use yaw, pitch, roll Co-authored-by: Ben Dichter <ben.dichter@gmail.com> * Add types and min/max for roll,pitch,yaw + fix typo for microephysChannels * Replace Pharmaceuticals with already existing fields and fix few more references * Make use of merged fancing for tsvs and add a note about spotted Pharmaceuticals * Remove some "bad latin" * Fix a link to be final, before redirects * using MACROS___make_sidecar_table * numbering * Adding icephys and ecephys to filename template * [ENH] Added microephys tag to electrode related enums * [ENH] Added additional channel enums for microphys * [ENH] Enlisted microphys channel types in type__channels * Various small fixes from code-review Co-authored-by: Lyuba Zehl <lyuba.zehl@ebrains.eu> Co-authored-by: Ben Dichter <ben.dichter@gmail.com> * [BEP032] Moved coord system related entities to correct yaml * Minor tuneups from code review (mostly on wording) Co-authored-by: Ben Dichter <ben.dichter@gmail.com> * A pass to harmonize to use "microephys" in favor over "ephys" Also used appropriate suffix in a few spots when encountered * Encode in schema that ATM we do require _electodes and _probes Also extended for microephys for channels and coordsystems * One more ephys -> microephys * Add .json to data (nwb and nix) and _probes files * [DATALAD RUNCMD] replace fancy unicode double quote with a simpler one === Do not change lines below === { "chain": [], "cmd": "sed -i -e 's,[“”],\",g' src/modality-specific-files/microelectrode-electrophysiology.md", "exit": 0, "extra_inputs": [], "inputs": [], "outputs": [], "pwd": "." } ^^^ Do not change lines above ^^^ * Address many remark lint issues using npx remark src/modality-specific-files/microelectrode-electrophysiology.md --frail --rc-path .remarkrc --output and then subselecting some changes and fixing some manually * One more tune up * items * [DATALAD RUNCMD] Fix up itemized things indentation === Do not change lines below === { "chain": [], "cmd": "sed -i -e 's,^\\([0-9]\\.\\|- \\) *,\\1 ,g' src/modality-specific-files/microelectrode-electrophysiology.md", "exit": 0, "extra_inputs": [], "inputs": [], "outputs": [], "pwd": "." } ^^^ Do not change lines above ^^^ * Address leftover of remark lint warnings * Remove custom paragraph on participants, replaced angles with yaw, pitch, roll Co-authored-by: Lyuba Zehl <lyuba.zehl@ebrains.eu> * [FIX] BEP032 - Correcting validations against real data example (#2132) * fix: schemapath resolution in Windows dev mode installation of bidsschematools * fix: schema validation through testing * chore: resolve conflict * fix: validation rules for events * doc: Reorder required metadata files for microephys sessions and clarify channel information * remove unneeded fields * move to task.yaml with __ * remove from microephys * Update src/modality-specific-files/microelectrode-electrophysiology.md Co-authored-by: Yaroslav Halchenko <debian@onerussian.com> * chore: linting * Add comprehensive stereotaxic coordinate documentation with visual guides - Replace x,y,z coordinates with standardized AP/ML/DV system - Add anatomical reference points (Bregma/Lambda) explanations - Include detailed angle measurement conventions with diagrams - Add 6 illustrative images for coordinate system understanding - Update electrode table example to use new coordinate naming * Add probe position columns for AP and ML axes and standardize units - Add AP__probes column for Anterior-Posterior probe positioning - Add ML__probes column for Medial-Lateral probe positioning - Add missing unit specification (mm) to existing DV__probes column * Update microelectrode electrophysiology documentation - Add ProbeInterface format links and JSON schema requirements for custom probes - Standardize requirement language (MUST/SHOULD) for coordsystem fields - Remove deprecated ProbeContours section * Updated probe tabular rule to match AP, ML, DV * Removed coordystem completely * Removes coordsys macro to fix failing mkdocs build * change the electrode positions back from AP/ML/DV to x,y,z * Update _probes.tsv example with standardized coordinate system and ProbeInterface fields Replace x/y/z coordinates with AP/ML/DV notation, add angle fields, and include probeinterface_manufacturer/model columns for better compatibility with ProbeInterface library * Update microephys entity rules and remove task/run requirements * Fix linter issues * Reorganized to match other modalities * Clarify DV axis description in microelectrode electrophysiology documentation * Update src/modality-specific-files/microelectrode-electrophysiology.md * Adds space entity for microephys electrodes * Add coordinate system metadata fields for microephys * Added rules to 1) override rule for coorsystem to be present for electrode.tsv 2) If coorsystem present, corresponding electrode.tsv must be present 3. Match the space between corresponding coordystem and electrodes files * Added entities for the photo specific to microephys * Rewrote the coordsystem JSON and photo section to align with ieeg but adopt to animal electrophysiology * Minor change to the default probe relative coordinate system * Probe interface exaple file tree * Add omission of microephys datatypes for overall channel file rule * Update src/modality-specific-files/microelectrode-electrophysiology.md * Update src/modality-specific-files/microelectrode-electrophysiology.md * Update src/modality-specific-files/microelectrode-electrophysiology.md * Fix indentation for note and example alerts * [MISC] Use Levels and custom json for probe interface files * Update src/modality-specific-files/microelectrode-electrophysiology.md * Update src/schema/objects/columns.yaml * Update src/modality-specific-files/microelectrode-electrophysiology.md * [MISC] Replace image syntax with HTML for anatomical reference diagrams * Fix formatting in example *_electrodes.tsv for improved readability * Apply suggestion from @bendichter * Remove channel_id def and use name__channels same as ieeg * Update microephys electrode.tsv to match ieeg * Changed probe_id to probe_name to avoid use of the concept ID against BIDS conventions * Add missing probe_height def and rewrite desc to match current markdown * Fixed minor typo failing schema validation * Fix remark linting issues * Added and modified texts for coordsystem requirements * Added icephys and ecephys distinction and restructured first section a little * Fixed initial_column to match AP, ML, DV * Update mock TSV and JSON and fix heading levels to match ieeg * Updated toy data section and links to example to match current spec * Minor formatting and style issues * Fix mock data tsv and JSON * Apply various wording suggestions from code review Co-authored-by: Ben Dichter <ben.dichter@gmail.com> * Apply suggestion from @yarikoptic Co-authored-by: Yaroslav Halchenko <debian@onerussian.com> * Apply suggestion from @bendichter * add back type info that was accidentally removed from whole_blood_radioactivity * Update link to iEEG stimulation documentation for clarity * Update src/modality-specific-files/microelectrode-electrophysiology.md * feat: add anatomical reference point to microelectrode electrophysiology schema * Fix up units and clarify that we need TODO work on the units/space more * Fix typo Update src/schema/objects/suffixes.yaml fixing a typo * rf: move "ProbeInterface Library" subsection down after probes.tsv * Revert "Apply suggestion from @bendichter" to bring back width__probes This reverts commit d1835cf . spotted by seeing na na in the table. We have height and depth so makes sense to keep width, and now we get ❯ git grep __probes -- src/schema/objects/columns.yaml src/schema/objects/columns.yaml:AP__probes: src/schema/objects/columns.yaml:depth__probes: src/schema/objects/columns.yaml:DV__probes: src/schema/objects/columns.yaml:height__probes: src/schema/objects/columns.yaml:hemisphere__probes: src/schema/objects/columns.yaml:manufacturer__probes: src/schema/objects/columns.yaml:material__probes: src/schema/objects/columns.yaml: name: material__probes src/schema/objects/columns.yaml:ML__probes: src/schema/objects/columns.yaml:type__probes: src/schema/objects/columns.yaml:width__probes: Ideally the rendered should have failed and not just added na's row but that shouldn't be fixed here... * docs: consolidate electrode coordinate system documentation Reorganize coordinate system documentation in microelectrode electrophysiology spec by removing standalone "Coordinate Systems for Electrode Positions" section and consolidating the information into a new "Electrode Position Coordinates" subsection under the electrodes description. This improves document flow and places coordinate guidance closer to the relevant table definition. * docs(appendices): add microelectrode surgical coordinates appendix Add new appendix documenting stereotaxic coordinate conventions for intracranial probe placement during surgery, including: - Anatomical reference points (Bregma and Lambda) - AP/ML/DV coordinate system conventions - Angle definitions for probe orientation (AP, ML, rotation) - Reference to BrainSTEM documentation source * Add probes related schema changes * docs: add microelectrode electrophysiology coordinate systems Add new section defining restricted keywords for MicroephysCoordinateSystem field in coordsystem.json for icephys and ecephys datasets. Includes: - Pixels (2D electrode localization) - Stereotaxic (generic animal neuroscience coordinates) - AllenCCFv3 (mouse brain reference space) - WaxholmSpace (rat brain coordinate system) - WistarRatAtlas * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * docs: convert RRID identifiers to hyperlinks in coordinate-systems * docs(ephys): add stream_id column documentation for channels.tsv Document the stream_id column format for linking channels to their corresponding data streams. Include specifications for: - NWB files with internal HDF5 paths to ElectricalSeries objects - NIX files referencing data arrays following NIX/Neo organization - Handling multiple data streams with comma-separated lists - Cross-file references using filename:path syntax * docs(ephys): clarify electrode name uniqueness requirement * Changed hemisphere cardinality from required to recommended * fix: correct typo in description for reference__microephys * fix: clarify electrode position coordinates for single electrode cases * fix: fix the name of matrial: material_probes -> material * fix: remove unnecessary brackets in coordsystem selectors * fix: change tables from pipe syntax to html syntax. This removes linter errors and is easier to maintain. * Fix yamllint and codespell issues * feat(microephys): add channel-level filtering columns and documentation - Add new Filtering Information section to microelectrode-electrophysiology.md explaining global and channel-specific filter parameters - Add software_filter_types column definition to columns.yaml for specifying filter types with Levels defined in channels.json - Add low_cutoff, high_cutoff, and notch optional columns to microephysChannels for channel-specific filter cutoff frequencies (consistent with iEEG spec) - Replace hardware_filters and software_filters columns with the new software_filter_types column in microephysChannels table * fix: add missing closing brace in JSON example for task description * fix: update description of MicroephysCoordinateSystem axes and link to surgical coordinates appendix * feat(schema): add model column for probes and fix electrode descriptions - Add new `model__probes` column to store probe model name/number (e.g., Neuropixels 1.0, A1x32-Poly3-5mm-25s-177) - Mark model__probes as recommended in microephysProbes rules - Fix typos in x__electrodes description ("there in" → "therein") - Clarify coordinate units reference in electrode position description * fix: update description for IntendedFor field in microephysCoordsystemGeneral * fix: correct name and type for associated_brain_region_id field in columns.yaml * fix: remove deprecated hardware_filters and software_filters columns in extracellular electrophysiology example * feat: add micrometer (um) as a valid unit for MicroephysCoordinateUnits * fix(schema): correct stereotaxic coordinate system axis definitions Update axis definitions in Stereotaxic enum to fix incorrect mappings: - X-axis (AP): Change positive direction from posterior to anterior - Y-axis: Change from DV to ML axis with right being positive - Z-axis: Change from ML to DV axis with ventral/inferior being positive * feat(microephys): add recording metadata fields to setup schema Add RecordingDuration, RecordingType, and EpochLength fields to the microephysSetup sidecar rules. RecordingDuration and RecordingType are recommended, while EpochLength is optional but recommended when RecordingType is "epoched". * docs(ephys): correct JSON metadata filename convention Update microelectrode electrophysiology documentation to use the correct file suffixes `*_ecephys.json` or `*_icephys.json` instead of the generic `*_ephys.json` to match the actual naming convention for extracellular and intracellular electrophysiology metadata files. * fix: correct pluralization in TODO comment for PharmaceuticalDoseUnits * fix(schema): update electrode position descriptions for clarity and consistency with space entities * fix(schema): rename x/y/z columns to x/y/z__electrodes in microephys Rename coordinate columns in microephysElectrodes table to include __electrodes suffix for disambiguation. Updates both initial_columns and columns sections to use the new naming convention. * docs(ephys): update coordinate system names and fix Stereotaxic spelling - Change StereoTaxic to Stereotaxic for correct scientific terminology - Replace PaxinosWatson with WaxholmSpace and WistarRatAtlas coordinate systems - Remove outdated TODO comment about adding coordinate systems to appendix - Update file naming examples to use corrected Stereotaxic spelling * feat(microephys): add brain atlas coordinate systems for rodents and primates Add new MicroephysCoordinateSystem options for commonly used stereotaxic atlases: - PaxinosWatson: rat brain atlas - FranklinPaxinos: mouse brain atlas - SwansonRat: rat brain atlas - CHARM: macaque cortical hierarchy atlas - D99: Saleem-Logothetis macaque atlas - PaxinosMacaque: macaque stereotaxic atlas - MarmosetBrainAtlas: marmoset brain atlas These additions expand support for microelectrophysiology recordings across multiple species with widely-used reference coordinate systems. * fix(linter): fix yamllint and remark errors * fix(schema): remove unused column defintions * Reverted commit and readded the removed type column for software_filters * fix: reconciling software_filter defintion with master * Update src/schema/objects/metadata.yaml * Update src/modality-specific-files/microelectrode-electrophysiology.md Co-authored-by: Reema Gupta <59512969+ree-gupta@users.noreply.github.com> * fix: microephys electrodes/coordsystem validation with space entity support * fix(convention): update the schema selector convention from == to using intersects * fix(convention): use __ instead of _ for microephys context * cleanup(schema): remove old unused probe suffix * cleanup(schema): remove unsused column contact_count * fix(convention): fix display name capitalization and hypenation * [BEP032] Review and clean-up suggestions (#2293) * enhancement(units): Remove dimension_unit and add default mm for probe * cleanup: remove resolved comment * cleanup: remove resolved comment * cleanup(schema): mark hemisphere__probes for future unification with base hemispher column * cleanup: comments about unifying probes and electrodes column * fix(schema): use um² for microephys electrode size Microelectrodes require micrometer-scale measurements different from iEEG. Added size__microephys with um² units. * fix(schema): remove use of dimension_unit in microephys * cleanup: remove unused surgery_date column (moved as comment to PR #1839) * fix(desc): include microephys for channel column in events * fix(typo): microeelectrode in events * refactor(schema): use base x/y/z columns with addendums for electrodes * change(units): remove unavailble dimension_unit reference and add um in accordance with ProbeInterface norms * fix(docs): add link to markdown for 2D coordinate systems * cleanup: remove resolved TODO comment by 6284d36 * cleanup(rule): remove unsused older check for coordystem file * future cleanup(schema): add a comment of a potential column desc unification * refactor(schema): use base unit column like iEEG for channels * Update src/modality-specific-files/microelectrode-electrophysiology.md Co-authored-by: Cody Baker <51133164+CodyCBakerPhD@users.noreply.github.com> * Update src/modality-specific-files/microelectrode-electrophysiology.md Co-authored-by: Thomas Wachtler <wachtler@bio.lmu.de> * Update src/modality-specific-files/microelectrode-electrophysiology.md Co-authored-by: Thomas Wachtler <wachtler@bio.lmu.de> * Update src/modality-specific-files/microelectrode-electrophysiology.md Co-authored-by: Thomas Wachtler <wachtler@bio.lmu.de> * Update src/modality-specific-files/microelectrode-electrophysiology.md Co-authored-by: Thomas Wachtler <wachtler@bio.lmu.de> * enh(schema): add electrode_name, make reference optional and generalize probe_name * Remove dimension_unit from electrode table description * Remove redundant statement about required columns order in *_electrodes.tsv file * move description of electrode position columns in microelectrode electrophysiology documentation * cleanup: remove additions to events that are not microephys specific * Remove outdated comments and streamline pharmaceuticals section in microelectrode electrophysiology documentation * update source attribution to BrainSTEM * fix(linter): fix remark linter error * fix(build): add missing closing ticks for TSV in md * Update src/modality-specific-files/microelectrode-electrophysiology.md Co-authored-by: Thomas Wachtler <wachtler@bio.lmu.de> * Update src/modality-specific-files/microelectrode-electrophysiology.md Co-authored-by: Thomas Wachtler <wachtler@bio.lmu.de> * docs(ephys): add electrode_name column to channels table example Add electrode_name column to the extracellular electrophysiology channels.tsv example to show the mapping between channels and their corresponding electrodes. * fix(microephys): move reference__microephys to be optional in channels schema * move reference__microephys * fix(coordinate-systems): update link format for Microelectrode Surgical Coordinates appendix to markdown * fix: Correct formatting in electrode data table for clarity * fix: Correct electrode name formatting in electrophysiology examples for consistency * docs: correct coordinate system descriptions and references - Fix WaxholmSpace species from rat to mouse and update DOI reference - Remove RRID links from SwansonRat, CHARM, D99, and MarmosetBrainAtlas - Add DOI link for SwansonRat reference - Correct PaxinosRhesusMonkey author list (Toga AW -> Evrard, H) * fix: Add ignore rule for SciCrunch URLs in link checker --------- Co-authored-by: Julia Sprenger <julia.sprenger@rwth-aachen.de> Co-authored-by: Yaroslav Halchenko <debian@onerussian.com> Co-authored-by: Horea Christian <chr@chymera.eu> Co-authored-by: Remi Gau <remi_gau@hotmail.com> Co-authored-by: Peyman Najafi <najafe.peyman@gmail.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Reema Gupta <59512969+ree-gupta@users.noreply.github.com> Co-authored-by: Ben Dichter <ben.dichter@gmail.com> Co-authored-by: Reema Gupta <reema.gupta@lmu.de> Co-authored-by: Lyuba Zehl <lyuba.zehl@ebrains.eu> Co-authored-by: Chris Markiewicz <markiewicz@stanford.edu> Co-authored-by: christinerogers <christinerogers@users.noreply.github.com> Co-authored-by: Thomas Wachtler <wachtler@bio.lmu.de>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2307 +/- ##
=======================================
Coverage 83.09% 83.09%
=======================================
Files 22 22
Lines 1698 1698
=======================================
Hits 1411 1411
Misses 287 287 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
@effigies it looks like there are minor merge conflicts. Should we resolve these? |
|
Yes. They look easy to resolve. |
|
Hey! @CodyCBakerPhD Thanks again for the PR. I found a potential issue/improvement in the spec: I missed adding Reference: bids-specification/src/schema/rules/files/raw/task.yaml Lines 10 to 14 in 4b40578 This would allow physiological recordings (cardiac, eye tracking) and stim to be stored alongside microelectrode electrophysiology data. I am still not sure if we can push to this PR branch or collect all the reviews for the end, hence not making changes yet. |
|
@bids-maintenance can we get this announced to the community through the mailing list for formal review, and get a few experts explicitly assigned/invited as potential reviewers? |
|
@bids-maintenance could you help us with the question: "can we get this announced to the community through the mailing list for formal review, and get a few experts explicitly assigned/invited as potential reviewers?" |
|
Hi, we have two scheduled BEP community review periods per year, the next one is in April. We put you on the list for this review period! Before that, some maintainers would like to meet with all of you and go through the BEP, to make sure everything is ready for the community review. @effigies and @rwblair, can you post a when2meet with your availabilities here? Thank you! |
|
While curating a set of resting-state ecephys datasets (NHP, no pharmacology, no stimuli), I noticed that the pharmaceutical fields introduced as Since they're recommended, omitting them still triggers Two possible fixes:
|
|
I think there is no specific rule, but @effigies struggled to move many RECOMMENDs into OPTIONAL for MRI since there was too much noise from validator. May be rule of thumb could be simple: What % of studies use some pharmacology worth adding? if majority (over 50%) -- keep in RECOMMENDED, otherwise - go for OPTIONAL. Eventually, with all the LLMing, and after underlying papers are associated with the dataset for mining and informed metadata editing like with WiP by @bendichter https://medit.dandiarchive.org for DANDI, those OPTIONAL could be auto-filled out or requested based on the study background mined from the extra materials. |
|
Another approach could be to add a required boolean field like |
|
Forgot to send a when2meet. @CodyCBakerPhD and anybody who wants to meet with us: https://www.when2meet.com/?35364181-QNHek |
Decision on 2026.03.04: Make it |
|
The other follow-up from the meeting with the BIDS maintainers related to very basic file content validation: bids-standard/bids-validator#369 |
Hi BIDS-maintainers :) do we have an update in which review period would this proposal fall in? Are there any more open questions or changes pending on our end? Tagging to notify: @julia-pfarr @effigies |
|
Was just going to send out emails. We're planning to open review on Monday. |
|
This PR is under review: #2406 Please post significant discussion to that thread. Minor issues, such as typos or clarifications, may be posted as suggestions to this PR. These suggestions may be accepted or rejected without discussion, so do not make substantial proposals in this way. |
The two probes.json `model` examples were bare key-value fragments, which the check-embedded-json hook cannot parse. Wrap each in braces to make them valid standalone JSON. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
effigies
left a comment
There was a problem hiding this comment.
Still working on a final review. These are my comments from reading the text. I haven't yet gone through the schema closely.
| MicroephysCoordinateUnits: | ||
| name: MicroephysCoordinateUnits | ||
| display_name: Microephys Coordinate Units | ||
| description: | | ||
| Units of the coordinates of `"MicroephysCoordinateSystem"`. | ||
| type: string | ||
| # TODO: Clarify if units need to be used or enums, if units, define pixels | ||
| enum: | ||
| - m | ||
| - mm | ||
| - cm | ||
| - um | ||
| - pixels |
There was a problem hiding this comment.
I don't understand this TODO. Apologies if I missed a discussion.
|
|
||
| Optional column names in `events.tsv` to support multiple recordings in a single data file: | ||
|
|
||
| <!-- TODO: Macro for events --> |
| Several real-world datasets have been formatted using this specification and can be used for practical guidance when curating a new dataset. | ||
| <!-- TODO: Update with current real datasets. A current version of these datasets [can be found on GIN](https://gin.g-node.org/NeuralEnsemble/BEP032-examples) . | ||
|
|
||
| For a complete dataset including all data samples the extracellular microelectrode dataset published in [Brochier (2018)](https://doi.org/10.1038/sdata.2018.55) has been reorganized according to the current version of this BEP, using the NIX data format. | ||
| The up-to-date version of the dataset [can be found on GIN](https://gin.g-node.org/sprenger/multielectrode_grasp/src/bep_animalephys) . | ||
|
|
||
| We will also publish another dataset using the NWB data format in the near future, and a dataset acquired --> |
There was a problem hiding this comment.
This TODO appears undone. If it is not ready, I would remove this section and open an issue to add it later.
There was a problem hiding this comment.
I think it came from good old times when also @JuliaSprenger was involved in this BEP, so that https://gin.g-node.org/sprenger/multielectrode_grasp/src/bep_animalephys was created. But overall it was left as TODO since it is a chicken-egg issue. We can't have this until we finalize BEP since otherwise those datasets might not be valid ones. I wonder also if we could/should just point to https://github.com/bids-dandisets/ which would have many of such examples. I would not bother with https://gin.g-node.org/NeuralEnsemble/BEP032-examples though since those are largely replaced with bids-examples. But I would leave in smth like
| Several real-world datasets have been formatted using this specification and can be used for practical guidance when curating a new dataset. | |
| <!-- TODO: Update with current real datasets. A current version of these datasets [can be found on GIN](https://gin.g-node.org/NeuralEnsemble/BEP032-examples) . | |
| For a complete dataset including all data samples the extracellular microelectrode dataset published in [Brochier (2018)](https://doi.org/10.1038/sdata.2018.55) has been reorganized according to the current version of this BEP, using the NIX data format. | |
| The up-to-date version of the dataset [can be found on GIN](https://gin.g-node.org/sprenger/multielectrode_grasp/src/bep_animalephys) . | |
| We will also publish another dataset using the NWB data format in the near future, and a dataset acquired --> | |
| Several real-world datasets have been formatted using this specification and can be used for practical guidance when curating a new dataset. | |
| <!-- TODO: Upon finalization of the BEP032, update with current real datasets if they are updated to the spec and pass bids-validator. | |
| For a complete dataset including all data samples the extracellular microelectrode dataset published in [Brochier (2018)](https://doi.org/10.1038/sdata.2018.55) has been reorganized according to the current version of this BEP, using the NIX data format and [hosted on GIN](https://gin.g-node.org/sprenger/multielectrode_grasp/src/bep_animalephys) . | |
| Also numerous datasets from the [DANDI Archive](https://dandiarchive.org) have been converted to BIDS using [nwb2bids]() and shared [on GitHub](https://github.com/bids-dandisets/). | |
| > |
There was a problem hiding this comment.
I think it came from good old times when also @JuliaSprenger was involved in this BEP, so that https://gin.g-node.org/sprenger/multielectrode_grasp/src/bep_animalephys was created. But overall it was left as TODO since it is a chicken-egg issue. We can't have this until we finalize BEP since otherwise those datasets might not be valid ones. I wonder also if we could/should just point to https://github.com/bids-dandisets/ which would have many of such examples. I would not bother with https://gin.g-node.org/NeuralEnsemble/BEP032-examples though since those are largely replaced with bids-examples. But I would leave in smth like
I confirm this historical point! The use of GIN to host such example datasets is probably obsolete now and any reference to it, at least the ones created by Julia Sprenger, should be removed... The TODO list is also outdated (same timeframe / investigator)...
There was a problem hiding this comment.
from what I know, the data available at https://gin.g-node.org/sprenger/multielectrode_grasp/src/bep_animalephys is old (last change 4 years ago), and therefore not compliant with the current state of the specs... I think @ree-gupta has produced a version of the same dataset (from Brochier 2018) that should be compliant with the current specs... we could simply replace the old link with the one that's up-to-date...
|
|
||
| This dataset contains intracellular data from slices acquired from two subjects (20220101-A and 20220101B). Details about the subjects and the sample generation are documented in the samples (tsv/json) files. Data of each subject is stored in separate subject directories (top level directories), each of which contains an 'icephys/' subdirectory. Note that there is no session-level directory in this case. Here, we choose the option of having "multiple tasks/runs in separate files" to demonstrate the high level of readability offered by the filenames in this case. | ||
|
|
||
| For the first subject only a single sample (a cell for patch-clamp terminology) was extracted (sample-cell001), on which three different protocol recordings were performed: two runs of current injection to characterize intrinsic properties, and one run of synaptic stimulation. The `scans.tsv` file stores information such as the starting recording times. The detailed information on the recording channel (such as the recording mode used) is stored in the `channels.tsv` which, in this case, is common to all available recordings. The probes and electrodes files provide information on the pipette and solutions used for the recordings and are also shared across data files. |
There was a problem hiding this comment.
The sample-<label> entity is first introduced here and only described in the photos section. As a reader it came out of nowhere.
This appears to have been taken from microscopy. I would reuse text from there to the extent that it's applicable.
Is samples.tsv required if the sample-<label> entity is used? Is it permitted?
There was a problem hiding this comment.
Hey BEP032 team, let's discuss @effigies 's point about sample-<label> coming out of nowhere. He's right that it's under-explained, and digging in I found the requirement level is inconsistent between the prose and the examples, plus a schema gap. Here's what I'd suggest, but curious what you all think:
Introduce the entity up front. Right now its only real definition is buried in the Photos section, framed as just for histological photos, but the intracellular examples use it on every file (sample-cell001, etc.). I'd propose adding a definition near the top in "Terminology: Modality and Datatypes," adapted from the microscopy text.
Split the requirement level by datatype rather than making it uniformly optional or copying microscopy's blanket REQUIRED:
ecephys(in-vivo extracellular): OPTIONAL, and typically absent, since there's no tissue sample. Requiring it here would be wrong.icephys(patch): REQUIRED. A patched cell effectively is a sample, and the datatype inherently deals in discrete cells/slices. This matches what our intracellular example already does (every file issample-cellNNN) and mirrors howmicrtreats it.
In the schema this is just leaving sample: optional for ecephys and changing it to sample: required in the icephys block of microephys.yaml.
One sub-question: micr does not exempt the single-sample case (one sample per subject still gets sample-01). I'd suggest we follow that, since a single-sample exemption causes filename churn if a second cell is added later and forces tools to special-case it. So: required for icephys even when there's only one sample.
samples.tsv. Chris also asked whether it's required when the entity is used. It currently isn't enforced at all for microephys, since SamplesTSVMissing only triggers on "micr" in dataset.modalities. I'd suggest requiring samples.tsv whenever the sample- entity is present (which, with the split above, means always for icephys and only-if-used for ecephys), and adding a check keyed on entity-presence rather than modality.
samples.tsv columns we inherit. These are modality-agnostic, so microephys gets them for free: sample_id (required), participant_id (required), sample_type (required), pathology (recommended), derived_from (recommended). They all fit microephys fine, so I don't think we need to define anything new. Two things worth a quick look:
sample_typeis a required enum, and its full value list is broader than just cell types:tissue,whole organisms,organoid,technical sample, pluscell line/in vitro differentiated cells/primary cell/cell-free sample/cloning host. So an acute slice maps cleanly totissueand organoid recordings toorganoid— no missing values. The only thing to settle is a granularity convention: for patch clamp, is the "sample" the slice or the individual cell? If it's the cell,tissueis still the closestsample_type. We could also register both and usederived_fromto linksample-cell001back tosample-slice01. I'd suggest we pick one convention and document it.sample_idlabels are restricted to^sample-[0-9a-zA-Z+]+$(no underscores/hyphens), sosample-cell001is fine but keep that in mind for examples.
Cleanup. Trim the Photos-section mention down to a back-reference to the new definition.
If the per-datatype split (optional ecephys / required icephys) and the entity-presence trigger for samples.tsv sound right, I'm happy to put up the edits. The main thing I'd want to settle first is the sample granularity convention (cell vs. slice) for patch data.
There was a problem hiding this comment.
nice dig! I do not know much about icephys, but is it really that there would be multiple samples per each subject? ATM we do mandate sub- entity already, so if it is actually typical to have multiple samples - people would just use it if/where needed to disambiguate within subject, so we do not even need to make it RECOMMENDED.
And yes, I would recommend having samples.tsv if _sample- entity is used.
NB/backref -- this will add one more "consistent" case for samples.tsv to be inconsistent' in having dual-index for a single plural entity filename as mentioned in #2283
There was a problem hiding this comment.
Yes, multiple samples per subject is normal for icephys. A single animal commonly yields several slices, and several cells are patched per slice, so >1 sample per subject is the rule, not the exception.
On the requirement level: sub- alone can't disambiguate samples within a subject, and as you note people would reach for sample- to do that anyway. So the real choice is whether sample- is present inconsistently (only when >1) or always. The problem with "only when needed" is churn: collect one cell → sub-X_..., collect a second later → you now have to rename the first file to add sample-, and tools have to special-case the single-sample form. micr avoids this by requiring sample- even for a single sample, and I'd lean toward icephys following it for the same reason.
Agreed on requiring samples.tsv whenever sample- is used.
That said, the required-vs-optional call (and the related cell-vs-slice granularity convention) has real trade-offs in both directions, so I think it warrants input from the larger BEP032 team before we settle it.
There was a problem hiding this comment.
Hey. Here my feedback.
Introduce the entity up front. I agree with your suggestion.
Split the requirement level by datatype It's good to make it OPTIONAL for ecephys (although "since there's no tissue sample" is not correct, just not the typical case). It should indeed be required for icephys.
requiring samples.tsv whenever sample- is used I would agree to that (I think this is the case already no?). From a metadata management point of view this is very annoying that this is not the case for subjects. Even if there is only one subject (or sample) you want to have structured information on that subject (or sample).
From my point of view all those cases can be true and should be possible to model in BIDS: one sample per subject, multiple samples per subject, one sample from multiple subjects, sample with no subject.
sample_type the enum list is indeed insufficient and should be extended/revised. We encountered this problem already for the atlas/microscopy people that started to use BIDS. However I'm not sure if they ever raised that issue officially. As inspiration, here is the list we use in openMINDS (which was usage driven so most likely also not complete).
There was a problem hiding this comment.
sample_type the enum list is indeed insufficient and should be extended/revised. We encountered this problem already for the atlas/microscopy people that started to use BIDS. However I'm not sure if they ever raised that issue officially. As inspiration, here is the list we use in openMINDS (which was usage driven so most likely also not complete).
Hi, I jump in a little bit (yes, I'm still around and following ;) ). Wanted to attend the meeting yesterday but finally couldn't...
I've had the same experience with the definition and use of sample for some microscopy data recently... With two issues:
- the enum list (I concurr with @lzehl , it should be extended/revised)
- the granularity question, which is in fact, if I'm not mistaken, not formally settled in the microscopy specs, leading to potential different interpretations of the micr specs (a case I encountered)
So I agree and support what you're doing here, in particular the addition of the text to define the granularity!
| ### Additional Procedure Information | ||
|
|
||
| Furthermore, additional information can be stored about the recording procedure. | ||
| We RECOMMEND to use a dedicated `Procedure` node with the following keys: | ||
|
|
||
| - `Pharmaceuticals` | ||
| - `Sample` | ||
| - `Supplementary` |
There was a problem hiding this comment.
This appears to be vestigial, as there is no Procedure metadata defined in the schema, and PharmaceuticalName is shown directly in the top-level metadata. (I'm assuming "node" means a field in a JSON structure.)
There was a problem hiding this comment.
Hey BEP032 team, @effigies is right that this it is very unclear what "node" refers to and I honestly don't remember. Prior to this we say "The *_ecephys.json or *_icephys.json file can be used to store any microephys-specific metadata for the dataset. All setup-related metadata should be stored in a dedicated node of the JSON file called Setup.". For me this reads as if we intended the *_ecephys.json or *_icephys.json to be nested jsons, that provide information of setup and procedure under highlevel keys"setup" and "procedure", but I think this is not true. Except for the filter metadata *_ecephys.json or *_icephys.json are supposed to be flat.
My suggestion: The node reference is confusing I would just delete it. Setup, Procedure (with Pharmaceuticals, Sample, Supplementary) are just subheaders to structure all possible keys of the *_ecephys.json and *_icephys.json sidecars.
This brings up though another general issue I see: for me the sample metadata in the *_ecephys.json and *_icephys.json sidecars (and actually in the microscopy sidecar JSON files) compared to what is/should be provided in the samples.tsv and samples.json is a bit unclear. I would have assumed that all in samples.tsv and samples.json refers to the unprocessed sample (so all that is true always for sample X, such as sample type, slice thickness, extraction protocol, etc) while the sample keys in the *_ecephys.json, *_icephys.json and microscopy JSON sidecars cover additional processes applied sequentially (such as staining, pharmaceuticals, etc) or sample related acquisition variables (such as deformation, etc) or information on tissue structures when there is no sample (such as examined body part in a typical ecephys case,or intravital microscopy settings). This seems to be not the logic though. Sorry to raise this now... I guess we cannot do much about this now, but maybe we can deliver good examples of what to define where and when for samples?
| suffixes: | ||
| - channels | ||
| extensions: | ||
| - .json | ||
| - .tsv |
There was a problem hiding this comment.
| suffixes: | |
| - channels | |
| extensions: | |
| - .json | |
| - .tsv |
Co-authored-by: Chris Markiewicz <markiewicz@stanford.edu>
effigies
left a comment
There was a problem hiding this comment.
Got through columns.yaml. Working my way in lexical order.
| The method used to identify the associated brain region (estimated|proof) | ||
| depending on anatomical pictures proofing the location or indirect estimation of the location. |
There was a problem hiding this comment.
@bendichter I think we just expect an enum here "estimated" or "proof". I would suggest to reformulate to:
The quality type of the method used to identify the associated brain region.
MUST be 'estimated' or 'proof'.
In openMINDS we call this criteria quality types and our terms here are 'asserted' (claim/subjective) vs 'processive' (repeatable process/objective).
| associated_brain_region: | ||
| name: associated_brain_region | ||
| display_name: Associated brain region | ||
| description: | | ||
| A textual indication on the location of the probe, | ||
| preferably species-independent terms as obtained, for example from Uberon. | ||
| type: string | ||
| associated_brain_region_id: | ||
| name: associated_brain_region_id | ||
| display_name: Associated brain region identifier | ||
| description: | | ||
| An identifier of the associated brain region based on the Uberon ontology | ||
| for anatomical structures in animals, for example "UBERON:0010415" | ||
| type: string |
There was a problem hiding this comment.
One of these seems to require Uberon, one uses Uberon as an example. If I were to read this as a curator, I would assume Uberon is required, and associated_brain_region is just vaguely worded.
What is the intent here?
There was a problem hiding this comment.
I think associated_brain_region and the corresponding associated_brain_region_id should not be limited to UBERON (which is only a generic cross species ontology). There are species specific anatomical atlas brain regions and their corresponding IDs out there which should be used if fitting (I don't understand why we say cross-species is preferred... not really a good advise). UBERON should just be an example. So the text needs revision. My sugestion:
associated_brain_region:
name: associated_brain_region
display_name: Associated brain region
description: |
A textual indication on the location of the probe,
preferably ontology-based, for example from UBERON.
type: string
associated_brain_region_id:
name: associated_brain_region_id
display_name: Associated brain region identifier
description: |
The corresponding persistent identifier of the associated brain region,
for example "UBERON:0010415".
type: string
| channel_label: | ||
| name: channel_label | ||
| display_name: Channel label | ||
| description: | | ||
| Human readable identifier. Use this name to specify the | ||
| content of signals not generated by electrodes. For example, | ||
| `DAQ internal synchronization signals`, `behavioral signals`, `behavioral cues`. | ||
| type: string |
There was a problem hiding this comment.
The examples seem to match more with what I would think of as a description than an identifier. How does this differ from description?
There was a problem hiding this comment.
@effigies human readable identifiers or maybe use better the term label here (?) differ from descriptions in multiple ways: shorter (best actually one word, max two words) and not necessarily explanatory. Descriptions should be fully formulated sentences actually explaining the thing or the content.
I think channel label rather corresponds to channel type. So I would remove channel label.
…d) fields for _channels.tsv Co-authored-by: Chris Markiewicz <effigies@gmail.com>
Co-authored-by: Chris Markiewicz <markiewicz@stanford.edu>
Various fixes to columns.yaml Co-authored-by: Chris Markiewicz <effigies@gmail.com>
|
I was inspired by @effigies's questions about the brain region columns to look more closely at how anatomical location is represented in this BEP, and I think there is a structural issue worth discussing as a group. Current SituationAnatomical location is annotated in two separate places. In Why This Is a ProblemThere are three issues, and they compound. First, a probe frequently spans multiple regions. A single shank can pass through cortex, hippocampus, and thalamus in one insertion, so one brain region per probe cannot describe where the recording actually happened. Second, localization really happens at the electrode level. Once you have histology or an atlas registration, what you recover is a region assignment per contact. The probe-level annotation is better understood as the intended target of the insertion than as the result of localization. Third, The third point is the one I would emphasize, because it is not only a matter of granularity. If the ontology term lives on the electrode, we can support different ontologies for different localizations, since different rows can carry identifiers from whichever atlas was actually used for that assignment. That flexibility is not really available at the probe level, where a single value has to stand for the whole insertion. Proposed ChangesFor For Once the columns are split this way, I anticipate the objection that the evidence method is usually constant across a probe's contacts, since there is typically one histological reconstruction per track, so the value would repeat down the whole column. I do not find that persuasive. Constant columns are routine in BIDS tabular files, and the method does vary in real data: contacts near a region boundary versus contacts deep inside a structure, contacts whose slice was damaged or lost, and assignments that were directly observed versus interpolated between two observed points. Per-contact seems like the safer granularity to me. If the group would rather avoid the repetition, the alternative worth considering is a sidecar JSON field, not leaving the column on I would also take the opportunity to fix two things about that column, which is the subject of @effigies's open thread on it. The name suggests a quality grade, but the concept is really how the region assignment was established, so something like I think this also resolves the question @effigies raised about whether Uberon is required or merely an example, since we would need to state the intent explicitly for the new electrode columns in any case. A few things I do not have a strong opinion on and would like input on:
|
|
Following up on @effigies's question about Current SituationThe column is Why the Species Condition Cannot Be EnforcedThe validator context does not expose species at all: Proposed Change@effigies asked whether it would be easier to just make the column required. That is close, but Instead, I propose requiring I would also drop "If not specified, |
|
@effigies asked how Current Situation
The examples in its description are the clearest sign of the overlap: Calling it an identifier makes the overlap worse rather than better, since the identifier for a channel is Proposed ChangeRemove The one reading that might justify a dedicated column is an alternative identifier that does not fit |


BEP 032: Under review
This BEP is currently under review: #2406
Please post significant discussion to that thread. Minor issues, such as typos or clarifications, may be posted as suggestions to this PR. These suggestions may be accepted or rejected without discussion, so do not make substantial proposals in this way.
Original post
To all @bids-standard/maintainers
In line with the penultimate steps outlined in the Proposed BEP Review process, the BEP032 team would like to submit our pull request for formal review by the maintainers and community!
This PR adds a specification for microelectrode electrophysiology datasets based on the BEP032 proposal.
We happily welcome all enhancements, fixes, suggestions, requests, and critiques by the broader community.
This work is the result of some enormous efforts over the last year by @ree-gupta @bendichter @yarikoptic @twachtler @robertoostenveld @lzehl @apdavison and many others whose full work history has been squashed as an extended description on this commit and can be seen in the original draft PR, a copy of which will be kept on a branch in perpetuity on my personal fork (https://github.com/CodyCBakerPhD/bids-specification/tree/backup-bep032-work).
Quick links