Repository: NeurodataWithoutBorders/nwbinspector (language: python)

Issue:
[Add Check]: Add Best Practice for External Mode usage for ImageSeries
### What would you like to see added to the NWBInspector?

Came up in meeting today - add best practice section covering external mode usage for natural behavioral videos saved as ImageSeries.

### Pseudocode or other logic of the check

_No response_

### Do you have any interest in helping implement the check function?

Yes.

### Code of Conduct

- [X] I agree to follow this project's [Code of Conduct](https://github.com/NeurodataWithoutBorders/nwbinspector/blob/dev/.github/CODE_OF_CONDUCT.rst)
- [X] Have you ensured this check was not already [requested](https://github.com/neurodatawithoutborders/nwbinspector/issues)?


Discussion / hints:
@h-mayorquin did you want to take this one?
I could, but where is the file that should be modified? Where does it live? 
Where is the repo for?
https://www.nwb.org/best-practices/
@h-mayorquin These are the actual current Best Practices: https://nwbinspector.readthedocs.io/en/dev/best_practices/best_practices_index.html

They are in the docs for this repo, let me know if you have trouble finding them.

@oruebel Should we replace the https://www.nwb.org/best-practices/ with a link to https://nwbinspector.readthedocs.io/en/dev/best_practices/best_practices_index.html?
@CodyCBakerPhD my best guess is that the issue is that even though  https://www.nwb.org/best-practices/ is no longer linked to from the website, Google still finds it as the top hit when searching for "NWB Best Practices". Unfortunately we can't hack the Google search. I'll update the old page on nwb.org to point to the best practice on the the NWB Inspector docs.

> my best guess is that the issue is that even though https://www.nwb.org/best-practices/ is no longer linked to from the website, Google still finds it as the top hit when searching for "NWB Best Practices".

Ahhhh, that'll do it.

Thanks for checking!
@CodyCBakerPhD I've now updated https://www.nwb.org/best-practices/ Please have a look and let me know if there are any further changes that I should make. 
I've now also added on nwb.org a link to the "Best Practices for NWB Data" to the [overview of NWB Policies page](https://www.nwb.org/policies-overview/) as well as in main menu as part of the Policies sub-menu. Hopefully that will help clarify this issue further and make the best practices easier to find from the nwb.org website:

<img width="1467" alt="Screen Shot 2022-08-30 at 12 09 10 PM" src="https://user-images.githubusercontent.com/10999845/187523309-17b3605d-d6d5-4815-8868-4a44f1e4f31f.png">


Fix patch:
diff --git a/.gitignore b/.gitignore
index 016afaa64..2cce8e938 100644
--- a/.gitignore
+++ b/.gitignore
@@ -21,10 +21,14 @@ dist/
 
 # VisualStudioCode
 .vscode/*
-!.vscode/settings.json
+.vscode/settings.json
 !.vscode/tasks.json
 !.vscode/extensions.json
 .history
 
+
 # MacOS
 .DS_store
+
+# Doc build
+/docs/_build/*
diff --git a/docs/api/checks.rst b/docs/api/checks.rst
index 70a460769..ee918f5fd 100644
--- a/docs/api/checks.rst
+++ b/docs/api/checks.rst
@@ -35,3 +35,7 @@ Optical Electrophysiology (ophys)
 Optogenetics (ogen)
 -------------------
 .. automodule:: nwbinspector.checks.ogen
+
+ImageSeries
+-----------
+.. automodule:: nwbinspector.checks.image_series
diff --git a/docs/best_practices/best_practices_index.rst b/docs/best_practices/best_practices_index.rst
index 8b1c983ee..a6c9db3da 100644
--- a/docs/best_practices/best_practices_index.rst
+++ b/docs/best_practices/best_practices_index.rst
@@ -26,5 +26,6 @@ Authors: Oliver Ruebel, Andrew Tritt, Ryan Ly, Cody Baker and Ben Dichter
    tables
    ecephys
    ogen
+   image_series
    simulated_data
    extensions
diff --git a/docs/best_practices/image_series.rst b/docs/best_practices/image_series.rst
new file mode 100644
index 000000000..3df4711f8
--- /dev/null
+++ b/docs/best_practices/image_series.rst
@@ -0,0 +1,26 @@
+ImageSeries
+===========
+
+Storage of ImageSeries
+----------------------
+
+.. _best_practice_image_series_file_too_large:
+
+Use external model when file is too large
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+
+When storing commmon multimedia formats using the :nwb-schema:ref:`sec-ImageSeries` the file should be stored as
+an external file. That is, the file should be packaged together with the nwb file instead of stored inside the format.
+This can be accomplished by using  the ``external_file`` file option to store the path instead.
+
+Check function: :py:meth:`~nwbinspector.checks.check_image_series_too_large`
+
+.. _best_practice_image_series_external_mode_path:
+
+Use relative path for external mode
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+When using ``external_file`` the paths passed in the ``external_file`` option should be relative to the location of the nwb file.
+
+Check function: :py:meth:`~nwbinspector.checks.check_image_series_external_file_relative`
diff --git a/nwbinspector/checks/image_series.py b/nwbinspector/checks/image_series.py
index 0f7bac44f..438582eea 100644
--- a/nwbinspector/checks/image_series.py
+++ b/nwbinspector/checks/image_series.py
@@ -3,7 +3,7 @@
 
 from pynwb.image import ImageSeries
 
-from ..register_checks import register_check, Importance, InspectorMessage
+from ..register_checks import register_check, Importance, InspectorMessage, Severity
 from ..tools import get_nwbfile_path_from_internal_object
 
 
@@ -38,3 +38,14 @@ def check_image_series_external_file_relative(image_series: ImageSeries):
                     "Please adjust the absolute path to be relative to the location of the NWBFile."
                 )
             )
+
+
+@register_check(importance=Importance.BEST_PRACTICE_VIOLATION, neurodata_type=ImageSeries)
+def check_image_series_too_large(image_series: ImageSeries, gb_lower_bound: float = 20.0):
+    """Check if an ImageSeries stored is larger than gb_lower_bound and suggests external file."""
+    data = image_series.data
+    data_size_gb = data.size * data.dtype.itemsize / 1e9
+    if data_size_gb > gb_lower_bound:
+        return InspectorMessage(
+            message=f"ImageSeries {image_series.name} is too large. Use external mode for storage",
+        )


Test patch:
diff --git a/tests/unit_tests/test_image_series.py b/tests/unit_tests/test_image_series.py
index 018111401..142a27c9e 100644
--- a/tests/unit_tests/test_image_series.py
+++ b/tests/unit_tests/test_image_series.py
@@ -12,8 +12,10 @@
     Importance,
     check_image_series_external_file_valid,
     check_image_series_external_file_relative,
+    check_image_series_too_large,
 )
 from nwbinspector.tools import make_minimal_nwbfile
+from nwbinspector.register_checks import Severity
 
 
 class TestExternalFileValid(TestCase):
@@ -125,3 +127,34 @@ def test_check_image_series_external_file_relative(self):
 def test_check_image_series_external_file_valid_pass_non_external():
     image_series = ImageSeries(name="TestImageSeries", rate=1.0, data=np.zeros(shape=(3, 3, 3, 3)), unit="TestUnit")
     assert check_image_series_external_file_valid(image_series=image_series) is None
+
+
+def test_check_small_image_series_stored_internally():
+    gb_size = 0.010  # 10 MB
+    frame_length = 10
+    total_elements = int(gb_size * 1e9 / np.dtype("float").itemsize) // (frame_length * frame_length)
+    data = np.zeros(shape=(total_elements, frame_length, frame_length, 1))
+    image_series = ImageSeries(name="ImageSeriesLarge", rate=1.0, data=data, unit="TestUnit")
+    assert check_image_series_too_large(image_series=image_series) is None
+
+
+def test_check_large_image_series_stored_internally():
+
+    gb_size = 0.010  # 10 MB
+    frame_length = 10
+    total_elements = int(gb_size * 1e9 / np.dtype("float").itemsize) // (frame_length * frame_length)
+    data = np.zeros(shape=(total_elements, frame_length, frame_length, 1))
+    image_series = ImageSeries(name="ImageSeriesLarge", rate=1.0, data=data, unit="TestUnit")
+    gb_lower_bound = gb_size * 0.9
+    inspector_message = check_image_series_too_large(image_series=image_series, gb_lower_bound=gb_lower_bound)
+
+    expected_message = InspectorMessage(
+        importance=Importance.BEST_PRACTICE_VIOLATION,
+        message=f"ImageSeries {image_series.name} is too large. Use external mode for storage",
+        check_function_name="check_image_series_too_large",
+        object_type="ImageSeries",
+        object_name="ImageSeriesLarge",
+        location="/",
+    )
+
+    assert inspector_message == expected_message