[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

Author: ree-gupta

src/schema/objects/columns.yaml

@@ -166,10 +166,11 @@ depth__probes:
   name: depth
   display_name: Depth
   description: |
-    Physical depth of the probe, for example, '0.3'.
+    Physical depth of the probe in mm, for example, '0.3'.
     This dimension should be omitted or set to 0 for two-dimensional (shank-type) probes.
     This dimension corresponds to the z-axis of the probe's local coordinate frame.
   type: number
+  unit: mm
 coordinate_system:
   name: coordinate_system
   display_name: Coordinate System
@@ -244,15 +245,6 @@ dimension:
     Size of the group (grid/strip/probe) that this electrode belongs to.
     Must be of form `[AxB]` with the smallest dimension first (for example, `[1x8]`).
   type: string
-# TODO: Add unit enums if everyone agrees to keep this column.
-dimension_unit:
-  name: dimension_unit
-  display_name: Dimension unit
-  description: |
-    Spatial units for a position or a physical dimension of electrodes, such as
-    'width', 'height' or 'depth' of the probe.
-    For example, `um`.
-  type: string
 duration:
   name: duration
   display_name: Event duration
@@ -297,7 +289,6 @@ filename:
     Relative paths to files.
   type: string
   format: participant_relative
-# TODO: Figure out and add rule for default values.
 gain:
   name: gain
   display_name: Gain
@@ -306,7 +297,6 @@ gain:
     to the signal stored in the data file.
     If no gain factor is provided it is assumed to be 1.
   type: number
-# TODO: Clarify if the optionality of the column be defined here or as an addendum in the rules.
 ground:
   name: ground
   display_name: Ground
@@ -369,10 +359,11 @@ height__probes:
   name: height
   display_name: Probe height
   description: |
-    Physical height of the probe, for example, '0.3'.
+    Physical height of the probe in mm, for example, '0.3'.
     This dimension should be omitted or set to 0 for one-dimensional (linear) probes.
     This dimension corresponds to the y-axis of the probe's local coordinate frame.
   type: number
+  unit: mm
 hemisphere:
   name: hemisphere
   display_name: Electrode hemisphere
@@ -382,7 +373,7 @@ hemisphere:
   enum:
     - $ref: objects.enums.left_hemisphere.value
     - $ref: objects.enums.right_hemisphere.value
-# TODO: Should the hemisphere details be unified for electrodes and probes or will it break other modality?
+# consider unifying with base `hemisphere` column between electrodes and probes
 hemisphere__probes:
   name: hemisphere
   display_name: Hemisphere
@@ -453,7 +444,7 @@ manufacturer:
     The manufacturer for each electrode.
     Can be used if electrodes were manufactured by more than one company.
   type: string
-# TODO: Should the manufacturer details be unified for electrodes and probes?
+# consider unifying with base `manufacturer` column between electrodes and probes
 manufacturer__probes:
   name: manufacturer
   display_name: Manufacturer
@@ -466,6 +457,7 @@ mapping:
   description: |
     Corresponding integer label in the standard BIDS label lookup.
   type: integer
+# consider unifying with base `material` column between electrodes and probes
 material:
   name: material
   display_name: Electrode material
@@ -834,7 +826,6 @@ short_channel:
     The total number of channels listed as short channels
     SHOULD be stored in `ShortChannelCount` in `*_nirs.json`.
   type: boolean
-# TODO: might need to dup for microephys__size and have it um^2
 signal_electrode:
   name: signal_electrode
   display_name: Signal electrode
@@ -849,6 +840,13 @@ size:
     Surface area of the electrode, units MUST be in `mm^2`.
   type: number
   unit: 'mm^2'
+size__microephys:
+  name: size
+  display_name: Electrode size
+  description: |
+    Surface area of the electrode, units MUST be in `um^2`.
+  type: number
+  unit: 'um^2'
 software_filters:
   name: software_filters
   display_name: Software filters
@@ -956,16 +954,6 @@ stream_id:
   description: |
     Data stream of the recording the signal.
   type: string
-# TODO: Clarify if column values can be arrays.
-surgery_date:
-  name: surgery_date
-  display_name: Surgery date
-  description: |
-    Date(s) of surgery. Datetime format and their anonymization
-    are described in [Units](SPEC_ROOT/common-principles.md#units).
-    The details of the surgery should be described in the sidecar json file.
-  type: string
-  format: datetime
 target_muscle:
   name: target_muscle
   display_name: Target muscle
@@ -1132,15 +1120,6 @@ units:
     (see [Units](SPEC_ROOT/common-principles.md#units)).
   type: string
   format: unit
-units__channels:
-  name: units
-  display_name: Units
-  description: |
-    Physical unit of the value represented in this channel,
-    for example, `V` for Volt, or `uV` for micro Volt
-    (see [Units](SPEC_ROOT/common-principles.md#units)).
-  type: string
-  format: unit
 units__nirs:
   name: units
   display_name: Units
@@ -1208,9 +1187,10 @@ width__probes:
   name: width
   display_name: Width
   description: |
-    Physical width of the probe, for example, '5'.
+    Physical width of the probe in mm, for example, '5'.
     This dimension corresponds to the x-axis of the probe's local coordinate frame.
   type: number
+  unit: mm
 whole_blood_radioactivity:
   name: whole_blood_radioactivity
   display_name: Whole blood radioactivity
@@ -1267,54 +1247,6 @@ z__optodes:
     - type: string
       enum:
         - n/a
-# TODO: Remove the overspecified entries for electrode positions if not used in microephys
-# TODO: finish up setup below
-x__electrodes:
-  name: x
-  display_name: Electrode X position
-  description: |
-    When no [`space-<label>`](SPEC_ROOT/appendices/entities.md#space) entity is used in the filename,
-    the position along the local width-axis relative to the probe origin
-    (see `coordinate_reference_point` in `*_probes.tsv`).
-    Units are specified by the `dimension_unit` column in `*_probes.tsv`.
-
-    When a `space-<label>` entity is used in the filename,
-    the position relative to the origin of the coordinate system along the first axis.
-    Units are specified by `MicroephysCoordinateUnits` in the corresponding `*_coordsystem.json` file.
-  type: number
-y__electrodes:
-  name: y
-  display_name: Electrode Y position
-  description: |
-    When no [`space-<label>`](SPEC_ROOT/appendices/entities.md#space) entity is used in the filename,
-    the position along the local height-axis relative to the probe origin
-    (see `coordinate_reference_point` in `*_probes.tsv`).
-    Units are specified by the `dimension_unit` column in `*_probes.tsv`.
-
-    When a `space-<label>` entity is used in the filename,
-    the position relative to the origin of the coordinate system along the second axis.
-    Units are specified by `MicroephysCoordinateUnits` in the corresponding `*_coordsystem.json` file.
-  type: number
-z__electrodes:
-  name: z
-  display_name: Electrode Z position
-  description: |
-    When no [`space-<label>`](SPEC_ROOT/appendices/entities.md#space) entity is used in the filename,
-    the position along the local depth-axis relative to the probe origin
-    (see `coordinate_reference_point` in `*_probes.tsv`).
-    Units are specified by the `dimension_unit` column in `*_probes.tsv`.
-
-    When a `space-<label>` entity is used in the filename,
-    the position relative to the origin of the coordinate system along the third axis.
-    Units are specified by `MicroephysCoordinateUnits` in the corresponding `*_coordsystem.json` file.
-
-    For 2D electrode localizations (for example, when the coordinate system is
-    `Pixels`), this SHOULD be a column of `n/a` values.
-  anyOf:
-    - type: number
-    - type: string
-      enum:
-        - n/a
 template_x:
   name: template_x
   display_name: X template position

src/schema/objects/metadata.yaml

@@ -2237,9 +2237,6 @@ MEGREFChannelCount:
     `MEGREFChannelCount` should be set to `0`.
   type: integer
   minimum: 0
-# TODO: Change if needs to be broken down into ecephys and icephys
-# TODO: Add links to the 2D coordinate system, Pixels and glossary entry.
-# TODO: Add enums for the valid values specific to microephys
 MicroephysCoordinateSystem:
   name: MicroephysCoordinateSystem
   display_name: Microephys Coordinate System
@@ -2253,7 +2250,8 @@ MicroephysCoordinateSystem:
     pixel indices in a 2D image (of either a volume-rendering,
     surface-rendering, operative photo, or operative drawing),
     this MUST be "Pixels". For more information, see the section
-    on 2D coordinate systems.
+    [allowed 2D coordinate systems](SPEC_ROOT/modality-specific-files/\
+    microelectrode-electrophysiology.md#allowed-2d-coordinate-systems).
     For a list of valid values for this field, see the associated glossary entry.
   anyOf:
     - $ref: objects.enums._MicroephysCoordSys

src/schema/rules/checks/channels.yaml

@@ -42,7 +42,7 @@ RequiredCoordsystem:
   selectors:
     - suffix == "electrodes"
     - extension == ".tsv"
-    - '!intersects([datatype], ["ecephys", "icephys"])' # TODO: need to verify and unify how selector is used EMG
+    - '!intersects([datatype], ["ecephys", "icephys"])'
     - datatype != "emg" # coordsys file may have space entity that electrodes.tsv lacks
   checks:
     - associations.coordsystem != null || associations.coordsystems != null

src/schema/rules/checks/microephys.yaml

@@ -1,19 +1,6 @@
 ---
-# For non-microephys modalities, electrodes.tsv always requires coordsystem.json
-RequiredCoordsystem:
-  issue:
-    code: REQUIRED_COORDSYSTEM
-    message: |
-      If an electrodes.tsv file is provided, an associated coordsystem.json must also be present.
-    level: error
-  selectors:
-    - suffix == "electrodes"
-    - extension == ".tsv"
-    - '!intersects([datatype], ["ecephys", "icephys"])'
-  checks:
-    - associations.coordsystem != null
-
 # For microephys, only space-labeled electrodes.tsv requires coordsystem.json
+# Non-microephys modalities are handled by RequiredCoordsystem in channels.yaml
 MicroephysRequiredCoordsystemWithSpace:
   issue:
     code: MICROEPHYS_COORDSYSTEM_REQUIRED

src/schema/rules/tabular_data/events.yaml

@@ -13,28 +13,28 @@ Events:
       level: optional
       description_addendum: |
         Note that this column only applies to data types where
-        channels are specified, such as EEG, iEEG, MEG or NIRS.
+        channels are specified, such as EEG, iEEG, MEG, NIRS or microephys.
 # TODO: Make the addendums more clear and descriptive
     data_entity_id:
       level: optional
       description_addendum: |
         Note that this column only applies to
-        microelectrode eleectrophysiology data type.
+        microelectrode electrophysiology data type.
     duration_index:
       level: optional
       description_addendum: |
         Note that this column only applies to
-        microelectrode eleectrophysiology data type.
+        microelectrode electrophysiology data type.
     onset_index:
       level: optional
       description_addendum: |
         Note that this column only applies to
-        microelectrode eleectrophysiology data type.
+        microelectrode electrophysiology data type.
     trial_id:
       level: optional
       description_addendum: |
         Note that this column only applies to
-        microelectrode eleectrophysiology data type.
+        microelectrode electrophysiology data type.
   additional_columns: allowed
   initial_columns:
     - onset

src/schema/rules/tabular_data/microephys.yaml

@@ -27,7 +27,6 @@ microephysProbes:
     width__probes: optional
     height__probes: optional
     depth__probes: optional
-    dimension_unit: optional
     rotation_angle: recommended
     coordinate_reference_point: recommended
     anatomical_reference_point: optional
@@ -48,19 +47,45 @@ microephysElectrodes:
   initial_columns:
     - name__electrodes
     - probe_name
-    - x__electrodes
-    - y__electrodes
-    - z__electrodes
+    - x
+    - y
+    - z
   columns:
     name__electrodes: required
     probe_name: required
-    x__electrodes: required
-    y__electrodes: required
-    z__electrodes: required
+    x:
+      level: required
+      description_addendum: |
+        When no [`space-<label>`](SPEC_ROOT/appendices/entities.md#space) entity is used in the filename,
+        the position along the local width-axis relative to the probe origin
+        (see `coordinate_reference_point` in `*_probes.tsv`) in micrometers (um).
+        When a `space-<label>` entity is used in the filename,
+        the position relative to the origin of the coordinate system along the first axis.
+        Units are specified by `MicroephysCoordinateUnits` in the corresponding `*_coordsystem.json` file.
+    y:
+      level: required
+      description_addendum: |
+        When no [`space-<label>`](SPEC_ROOT/appendices/entities.md#space) entity is used in the filename,
+        the position along the local height-axis relative to the probe origin
+        (see `coordinate_reference_point` in `*_probes.tsv`) in micrometers (um).
+        When a `space-<label>` entity is used in the filename,
+        the position relative to the origin of the coordinate system along the second axis.
+        Units are specified by `MicroephysCoordinateUnits` in the corresponding `*_coordsystem.json` file.
+    z:
+      level: required
+      description_addendum: |
+        When no [`space-<label>`](SPEC_ROOT/appendices/entities.md#space) entity is used in the filename,
+        the position along the local depth-axis relative to the probe origin
+        (see `coordinate_reference_point` in `*_probes.tsv`) in micrometers (um).
+        When a `space-<label>` entity is used in the filename,
+        the position relative to the origin of the coordinate system along the third axis.
+        Units are specified by `MicroephysCoordinateUnits` in the corresponding `*_coordsystem.json` file.
+        For 2D electrode localizations (for example, when the coordinate system is
+        `Pixels`), this SHOULD be a column of `n/a` values.
     hemisphere: recommended
     impedance: recommended
     shank_id: optional
-    size: optional
+    size__microephys: optional
     electrode_shape: optional
     material: optional
     location: recommended
@@ -79,13 +104,13 @@ microephysChannels:
     - name__channels
     - reference__microephys
     - type__channels
-    - units__channels
+    - units
     - sampling_frequency
   columns:
     name__channels: required
     reference__microephys: required
     type__channels: required
-    units__channels: required
+    units: required
     sampling_frequency: optional
     low_cutoff: optional
     high_cutoff: optional