From 5419a51fc7570b8e6aeebc83371331b8ea49f8dc Mon Sep 17 00:00:00 2001 From: Ben Dichter Date: Tue, 30 Jun 2026 16:52:40 -0400 Subject: [PATCH 1/2] Add check_publication_list_format check Split out from #651 per review feedback: detect comma-separated DOIs/URLs in related_publications entries that should be separate list entries. Severity set to BEST_PRACTICE_VIOLATION as suggested in review. Refs #419 Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 1 + docs/best_practices/nwbfile_metadata.rst | 7 +- src/nwbinspector/checks/__init__.py | 2 + src/nwbinspector/checks/_nwbfile_metadata.py | 29 +++++ tests/unit_tests/test_nwbfile_metadata.py | 105 +++++++++++++++++++ 5 files changed, 143 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8cccbc13..443899e5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,6 +57,7 @@ * Added `check_image_series_starting_frame_without_external_file` to verify that `starting_frame` is not set when `external_file` is not used in an `ImageSeries`. [#235](https://github.com/NeurodataWithoutBorders/nwbinspector/issues/235) * Added `check_sweeptable_deprecated` to detect usage of the deprecated `SweepTable` in NWB files with schema version >= 2.4.0, which should use `IntracellularRecordingsTable` instead. [#657](https://github.com/NeurodataWithoutBorders/nwbinspector/issues/657) * Added `check_time_series_data_is_not_empty` to detect empty `.data` fields in `TimeSeries` containers, which often indicate incomplete data entry or conversion errors. Skips `ImageSeries` with `external_file` set, where empty data is intentional. [#668](https://github.com/NeurodataWithoutBorders/nwbinspector/pull/668) +* Added `check_publication_list_format` to detect comma-separated DOIs/URLs in `related_publications` entries that should be separate list entries. [#419](https://github.com/NeurodataWithoutBorders/nwbinspector/issues/419) ### Improvements * Added documentation to API and CLI docs on how to use the dandi config option. [#624](https://github.com/NeurodataWithoutBorders/nwbinspector/pull/624) diff --git a/docs/best_practices/nwbfile_metadata.rst b/docs/best_practices/nwbfile_metadata.rst index 2678fc2e..c8d987bb 100644 --- a/docs/best_practices/nwbfile_metadata.rst +++ b/docs/best_practices/nwbfile_metadata.rst @@ -181,7 +181,12 @@ of the form ``'doi: ###'`` or as an external link of the form ``'http://dx.doi.o This allows metadata collection programs, such as those on the :dandi-archive:`DANDI archive <>` to easily form direct hyperlinks to the publications. -Check function: :py:meth:`~nwbinspector.checks._nwbfile_metadata.check_doi_publications` +Each publication should be a separate entry in the list. Do not combine multiple DOIs or URLs into a single +comma-separated string. For example, use ``["https://doi.org/10.1234/abc", "https://doi.org/10.5678/def"]`` instead of +``["https://doi.org/10.1234/abc,https://doi.org/10.5678/def"]``. + +Check functions: :py:meth:`~nwbinspector.checks._nwbfile_metadata.check_doi_publications` and +:py:meth:`~nwbinspector.checks._nwbfile_metadata.check_publication_list_format` diff --git a/src/nwbinspector/checks/__init__.py b/src/nwbinspector/checks/__init__.py index d7a386a6..8b5beac0 100644 --- a/src/nwbinspector/checks/__init__.py +++ b/src/nwbinspector/checks/__init__.py @@ -55,6 +55,7 @@ check_institution, check_keywords, check_processing_module_name, + check_publication_list_format, check_session_id_no_slashes, check_session_start_time_future_date, check_session_start_time_old_date, @@ -151,6 +152,7 @@ "check_session_start_time_future_date", "check_processing_module_name", "check_session_start_time_old_date", + "check_publication_list_format", "check_optogenetic_stimulus_site_has_optogenetic_series", "check_excitation_lambda_in_nm", "check_plane_segmentation_image_mask_shape_against_ref_images", diff --git a/src/nwbinspector/checks/_nwbfile_metadata.py b/src/nwbinspector/checks/_nwbfile_metadata.py index 5333714c..0e4a0d3e 100644 --- a/src/nwbinspector/checks/_nwbfile_metadata.py +++ b/src/nwbinspector/checks/_nwbfile_metadata.py @@ -168,6 +168,35 @@ def check_doi_publications(nwbfile: NWBFile) -> Optional[Iterable[InspectorMessa return None +@register_check(importance=Importance.BEST_PRACTICE_VIOLATION, neurodata_type=NWBFile) +def check_publication_list_format(nwbfile: NWBFile) -> Optional[Iterable[InspectorMessage]]: + """ + Check if related_publications entries contain comma-separated values that should be separate list entries. + + Best Practice: :ref:`best_practice_doi_publications` + """ + if not nwbfile.related_publications: + return None + for publication in nwbfile.related_publications: + publication = publication.decode() if isinstance(publication, bytes) else publication + # Check for comma-separated DOIs or URLs within a single entry + # Look for patterns like "doi:xxx,doi:yyy" or "https://doi.org/xxx,https://doi.org/yyy" + if "," in publication: + # Check if the comma appears to separate multiple DOIs/URLs + parts = [p.strip() for p in publication.split(",")] + doi_indicators = ["doi:", "doi.org/", "dx.doi.org/"] + doi_like_parts = [part for part in parts if any(indicator in part.lower() for indicator in doi_indicators)] + if len(doi_like_parts) > 1: + yield InspectorMessage( + message=( + f"Metadata /general/related_publications contains a comma-separated list '{publication}'. " + "Each publication should be a separate entry in the list, not combined in a single string." + ) + ) + + return None + + @register_check(importance=Importance.CRITICAL, neurodata_type=Subject) def check_subject_age(subject: Subject) -> Optional[InspectorMessage]: """Check if the Subject age is in ISO 8601 or our extension of it for ranges.""" diff --git a/tests/unit_tests/test_nwbfile_metadata.py b/tests/unit_tests/test_nwbfile_metadata.py index af988d87..53a12fcf 100644 --- a/tests/unit_tests/test_nwbfile_metadata.py +++ b/tests/unit_tests/test_nwbfile_metadata.py @@ -23,6 +23,7 @@ check_institution, check_keywords, check_processing_module_name, + check_publication_list_format, check_session_id_no_slashes, check_session_start_time_future_date, check_session_start_time_old_date, @@ -277,6 +278,110 @@ def test_check_doi_publications_multiple_fail(): ] +def test_check_publication_list_format_pass(): + """Test that properly formatted publications pass the check.""" + nwbfile = NWBFile( + session_description="", + identifier=str(uuid4()), + session_start_time=datetime.now().astimezone(), + related_publications=["https://doi.org/10.1234/abc", "https://doi.org/10.5678/def"], + ) + assert check_publication_list_format(nwbfile) is None + + +def test_check_publication_list_format_pass_single_comma_in_title(): + """Test that a single publication with a comma in the title passes (not multiple DOIs).""" + nwbfile = NWBFile( + session_description="", + identifier=str(uuid4()), + session_start_time=datetime.now().astimezone(), + related_publications=["Some publication title, with comma"], + ) + assert check_publication_list_format(nwbfile) is None + + +def test_check_publication_list_format_pass_no_publications(): + """Test that no related_publications passes the check.""" + nwbfile = NWBFile( + session_description="", + identifier=str(uuid4()), + session_start_time=datetime.now().astimezone(), + ) + assert check_publication_list_format(nwbfile) is None + + +def test_check_publication_list_format_fail_comma_separated_doi_urls(): + """Test detection of comma-separated DOI URLs in a single entry.""" + nwbfile = NWBFile( + session_description="", + identifier=str(uuid4()), + session_start_time=datetime.now().astimezone(), + related_publications=["https://doi.org/10.1234/abc,https://doi.org/10.5678/def"], + ) + assert check_publication_list_format(nwbfile) == [ + InspectorMessage( + message=( + "Metadata /general/related_publications contains a comma-separated list " + "'https://doi.org/10.1234/abc,https://doi.org/10.5678/def'. " + "Each publication should be a separate entry in the list, not combined in a single string." + ), + importance=Importance.BEST_PRACTICE_VIOLATION, + check_function_name="check_publication_list_format", + object_type="NWBFile", + object_name="root", + location="/", + ) + ] + + +def test_check_publication_list_format_fail_comma_separated_doi_prefix(): + """Test detection of comma-separated DOI prefixes in a single entry.""" + nwbfile = NWBFile( + session_description="", + identifier=str(uuid4()), + session_start_time=datetime.now().astimezone(), + related_publications=["doi:10.1234/abc, doi:10.5678/def"], + ) + assert check_publication_list_format(nwbfile) == [ + InspectorMessage( + message=( + "Metadata /general/related_publications contains a comma-separated list " + "'doi:10.1234/abc, doi:10.5678/def'. " + "Each publication should be a separate entry in the list, not combined in a single string." + ), + importance=Importance.BEST_PRACTICE_VIOLATION, + check_function_name="check_publication_list_format", + object_type="NWBFile", + object_name="root", + location="/", + ) + ] + + +def test_check_publication_list_format_bytestring_fail(): + """Test that bytestrings are properly decoded and checked.""" + nwbfile = NWBFile( + session_description="", + identifier=str(uuid4()), + session_start_time=datetime.now().astimezone(), + related_publications=[b"https://doi.org/10.1234/abc,https://doi.org/10.5678/def"], + ) + assert check_publication_list_format(nwbfile) == [ + InspectorMessage( + message=( + "Metadata /general/related_publications contains a comma-separated list " + "'https://doi.org/10.1234/abc,https://doi.org/10.5678/def'. " + "Each publication should be a separate entry in the list, not combined in a single string." + ), + importance=Importance.BEST_PRACTICE_VIOLATION, + check_function_name="check_publication_list_format", + object_type="NWBFile", + object_name="root", + location="/", + ) + ] + + def test_check_subject_sex(): nwbfile = NWBFile(session_description="", identifier=str(uuid4()), session_start_time=datetime.now().astimezone()) nwbfile.subject = Subject(subject_id="001") From 043e3eedcec2e82a217ad2ea9f509ab5b72a54e1 Mon Sep 17 00:00:00 2001 From: Ben Dichter Date: Tue, 30 Jun 2026 17:26:03 -0400 Subject: [PATCH 2/2] Update src/nwbinspector/checks/_nwbfile_metadata.py Co-authored-by: Ryan Ly <310197+rly@users.noreply.github.com> --- src/nwbinspector/checks/_nwbfile_metadata.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/nwbinspector/checks/_nwbfile_metadata.py b/src/nwbinspector/checks/_nwbfile_metadata.py index 0e4a0d3e..cf477d1a 100644 --- a/src/nwbinspector/checks/_nwbfile_metadata.py +++ b/src/nwbinspector/checks/_nwbfile_metadata.py @@ -184,7 +184,7 @@ def check_publication_list_format(nwbfile: NWBFile) -> Optional[Iterable[Inspect if "," in publication: # Check if the comma appears to separate multiple DOIs/URLs parts = [p.strip() for p in publication.split(",")] - doi_indicators = ["doi:", "doi.org/", "dx.doi.org/"] + doi_indicators = ["doi:", "doi.org/"] doi_like_parts = [part for part in parts if any(indicator in part.lower() for indicator in doi_indicators)] if len(doi_like_parts) > 1: yield InspectorMessage(