[Best Practices] Add check for TimeSeries unit follows CMIXF-12 convention

docs/best_practices/best_practices_index.rst

@@ -29,3 +29,4 @@ Authors: Oliver Ruebel, Andrew Tritt, Ryan Ly, Cody Baker and Ben Dichter
    image_series
    simulated_data
    extensions
+   units

docs/best_practices/time_series.rst

@@ -37,7 +37,10 @@ measurement for that data, using the appropriate type from the
 
 Check function: :py:meth:`~nwbinspector.checks.time_series.check_missing_unit`
 
+It is recommended that the **formatting** of SI units be `CMIXF-12 <https://people.csail.mit.edu/jaffer/MIXF/CMIXF-12>`_ compliant.
+See :ref:`units` for more information.
 
+Check function: :py:meth:`~nwbinspector.checks.time_series.check_unit_formatting`
 
 .. _best_practice_time_series_global_time_reference:
 

docs/best_practices/units.rst

@@ -0,0 +1,66 @@
+.. _units:
+
+Units Formatting
+================
+
+As described in the :ref:`best_practice_unit_of_measurement`,
+the specification of units should follow the International System of Units (SI).
+
+The `CMIXF-12 <https://people.csail.mit.edu/jaffer/MIXF/CMIXF-12>`_ convention for encoding
+SI units is recommended to minimize the variability of representation of unit values.
+If the unit of measurement is unknown or unavailable, the unit can be declared as "a.u."
+which stands for arbitrary units.
+Please note the appropriate upper- or lower- casing when using CMIXF-12.
+
+The `cmixf <https://github.com/sensein/cmixf>`_ Python package is used to check whether
+the formatting of unit is compliant.
+
+Unit table
+^^^^^^^^^^
+.. list-table::
+   :widths: 25 25 50
+   :align: center
+   :header-rows: 1
+
+   * - Neurodata type
+     - Unit in CMIXF-12
+     - Unit Name
+   * - :py:class:`~pynwb.behavior.SpatialSeries`
+     - "m"
+     - Meters
+   * - :py:class:`~pynwb.ecephys.ElectricalSeries`
+     - "V"
+     - Volts
+   * - :py:class:`~pynwb.ecephys.SpikeEventSeries`
+     - "V"
+     - Volts
+   * - :py:class:`~pynwb.icephys.PatchClampSeries`
+     - "V"
+     - Volts
+   * - :py:class:`~pynwb.icephys.CurrentClampSeries`
+     - "V"
+     - Volts
+   * - :py:class:`~pynwb.icephys.IZeroClampSeries`
+     - "V"
+     - Volts
+   * - :py:class:`~pynwb.icephys.CurrentClampStimulusSeries`
+     - "A"
+     - Amperes
+   * - :py:class:`~pynwb.icephys.VoltageClampSeries`
+     - "A"
+     - Amperes
+   * - :py:class:`~pynwb.icephys.VoltageClampStimulusSeries`
+     - "V"
+     - Volts
+   * - :py:class:`~pynwb.image.ImageSeries`
+     - "a.u."
+     - Arbitrary Units (unknown)
+   * - :py:class:`~pynwb.image.IndexSeries`
+     - "a.u."
+     - Arbitrary Units (unknown)
+   * - :py:class:`~pynwb.image.ImageMaskSeries`
+     - "a.u."
+     - Arbitrary Units (unknown)
+   * - :py:class:`~pynwb.image.OpticalSeries`
+     - "a.u."
+     - Arbitrary Units (unknown)

nwbinspector/checks/time_series.py

@@ -2,6 +2,7 @@
 import numpy as np
 
 from pynwb import TimeSeries
+from cmixf.parser import CMIXFLexer
 
 # from ..tools import all_of_type
 from ..register_checks import register_check, Importance, Severity, InspectorMessage
@@ -92,3 +93,23 @@ def check_resolution(time_series: TimeSeries):
         return InspectorMessage(
             message=f"'resolution' should use -1.0 or NaN for unknown instead of {time_series.resolution}."
         )
+
+
+@register_check(importance=Importance.BEST_PRACTICE_VIOLATION, neurodata_type=TimeSeries)
+def check_unit_formatting(time_series: TimeSeries):
+    """
+    Check the unit value of a TimeSeries that it complies with CMIXF-12 convention for formatting the units.
+
+    Best Practice: :ref:`best_practice_units_of_measurement`
+    """
+
+    # Early return for arbitrary units that are unknown or unavailable.
+    if time_series.unit == "a.u.":
+        return
+
+    lexer = CMIXFLexer()
+    tokens = lexer.tokenize(time_series.unit)
+    try:
+        list(tokens)
+    except ValueError:
+        return InspectorMessage(message=f"The 'unit' should adhere to CMIXF-12 format instead of '{time_series.unit}'.")

requirements.txt

@@ -7,3 +7,4 @@ click
 tqdm
 numpy>=1.20.0,<1.21.0;python_version<'3.8'
 numpy>=1.22.0,<1.23.0;python_version>='3.8'  # <1.23 bound is from PyNWB
+cmixf

tests/unit_tests/test_time_series.py

@@ -12,6 +12,7 @@
     check_timestamps_ascending,
     check_missing_unit,
     check_resolution,
+    check_unit_formatting,
 )
 from nwbinspector.testing import check_streaming_tests_enabled
 from nwbinspector.utils import get_package_version, robust_s3_read
@@ -202,3 +203,50 @@ def test_check_resolution_fail():
         object_name="test",
         location="/",
     )
+
+
+def test_check_data_unit_fail():
+    time_series = pynwb.TimeSeries(
+        name="test",
+        unit="test_unit",
+        data=[1, 2, 3],
+        timestamps=[1, 2, 3],
+    )
+    assert check_unit_formatting(time_series) == InspectorMessage(
+        message="The 'unit' should adhere to CMIXF-12 format instead of 'test_unit'.",
+        importance=Importance.BEST_PRACTICE_VIOLATION,
+        check_function_name="check_unit_formatting",
+        object_type="TimeSeries",
+        object_name="test",
+        location="/",
+    )
+
+
+def test_check_data_unit_pass():
+    time_series = pynwb.TimeSeries(
+        name="test",
+        unit="m",
+        data=[1, 2, 3],
+        timestamps=[1, 2, 3],
+    )
+    assert check_unit_formatting(time_series) is None
+
+
+def test_check_data_unit_with_derived_unit():
+    time_series = pynwb.TimeSeries(
+        name="test",
+        unit="m/s",
+        data=[1, 2, 3],
+        timestamps=[1, 2, 3],
+    )
+    assert check_unit_formatting(time_series) is None
+
+
+def test_check_data_unit_with_arbitrary_unit():
+    time_series = pynwb.TimeSeries(
+        name="test",
+        unit="a.u.",
+        data=[1, 2, 3],
+        timestamps=[1, 2, 3],
+    )
+    assert check_unit_formatting(time_series) is None