Skip to content

Corrected the labeling notification to proper BIDS #176

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
CodyCBakerPhD merged 14 commits into from
Nov 19, 2025
Merged

Corrected the labeling notification to proper BIDS #176

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
8449e03
fix: correct the labeling notification to proper BIDS
CodyCBakerPhD Nov 4, 2025
c3a43fc
Merge branch 'main' into fix_label_notification
CodyCBakerPhD Nov 6, 2025
3cf62bc
Merge branch 'main' into fix_label_notification
CodyCBakerPhD Nov 7, 2025
a1f87c6
fix bad merge conflict
CodyCBakerPhD Nov 7, 2025
88b0e28
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Nov 7, 2025
84ede46
Merge branch 'main' into fix_label_notification
CodyCBakerPhD Nov 8, 2025
6d32c12
Fix bad merge
CodyCBakerPhD Nov 8, 2025
9443073
fix: bad merge
Nov 10, 2025
59c8be6
Merge branch 'main' into fix_label_notification
CodyCBakerPhD Nov 12, 2025
6ca7f54
Merge branch 'main' into fix_label_notification
CodyCBakerPhD Nov 12, 2025
e9c7d17
Merge branch 'main' into fix_label_notification
CodyCBakerPhD Nov 17, 2025
a812243
Apply suggestion from @candleindark
CodyCBakerPhD Nov 18, 2025
506a59a
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Nov 18, 2025
e3e7bd2
Update OS list for remote tests in workflow
CodyCBakerPhD Nov 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions src/nwb2bids/_command_line_interface/_main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

from .._converters._run_config import RunConfig
from .._core._convert_nwb_dataset import convert_nwb_dataset
from .._inspection._inspection_result import Severity
from .._tools._pluralize import _pluralize


Expand Down Expand Up @@ -121,3 +122,11 @@ def _run_convert_nwb_dataset(

if console_notification != "" and not silent:
rich_click.echo(message=console_notification)

not_any_failures = not notifications or not any(
notification.severity == Severity.ERROR for notification in notifications
Copy link
Contributor

@candleindark candleindark Nov 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do we do with the case notification.severity == Severity.CRITICAL

Copy link
Collaborator Author

@CodyCBakerPhD CodyCBakerPhD Nov 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those more directly relate to the case where nwb2bids produces an 'invalid' BIDS directory

I think in a follow-up we can maybe warn that this might be the case, though until various BEPs get merged on BIDS side, this warning will nearly always occur so maybe I'd not add that until things stabilize a bit upstream

Copy link
Contributor

@candleindark candleindark Nov 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those more directly relate to the case where nwb2bids produces an 'invalid' BIDS directory

If this is the case, I think we are treading a very fine line here. We are binding the validity of the resulting BIDS directory on the severity of a notification while the severity is more intuitively interpreted as the "severity" of the notification being raised. Also, this special binding is not documented in

nwb2bids/src/nwb2bids/_inspection/_inspection_result.py

Lines 27 to 41 in 0f03ccf

@enum.unique
class Severity(enum.Enum):
"""
Quantifier of relative severity (how important it is to resolve) for inspection results.
The larger the value, the more critical it is.
If an issue can be categorized in multiple ways, the most severe category should be chosen.
"""
INFO = enum.auto() # Not an indication of problem but information of status or confirmation
HINT = enum.auto() # Data is valid but could be improved
WARNING = enum.auto() # Data is not recognized as valid. Changes are needed to ensure validity
ERROR = enum.auto() # Data is recognized as invalid
CRITICAL = enum.auto() # A serious invalidity in data
.

I am not proposing to provide the documentation since I believe the severity level should only be used to signify how severe the notification being raised is. I am proposing to include the notification.severity == Severity.CRITICAL condition. I.e. instead of

any(
        notification.severity == Severity.ERROR for notification in notifications
    )

we should have

any(
        notification.severity == Severity.ERROR  or notification.severity == Severity.CRITICAL for notification in notifications
    )

After all, if the resulting BIDS dataset is invalid, I don't think we should print the message "BIDS dataset was successfully created!"

Copy link
Collaborator Author

@CodyCBakerPhD CodyCBakerPhD Nov 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good - it is a separate enhancement to the main topic of this PR however (and would benefit from even more targeted improvements + testing on separate PR) opened #193

)
if not_any_failures and not silent:
text = "BIDS dataset was successfully created!"
console_notification = rich_click.style(text=text, fg="green")
rich_click.echo(message=console_notification)
13 changes: 7 additions & 6 deletions src/nwb2bids/bids_models/_bids_session_metadata.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,14 +71,15 @@ def _check_fields(self, file_paths: list[pathlib.Path] | list[pydantic.HttpUrl])
title="Invalid session ID",
reason=(
"The session ID contains invalid characters. "
"BIDS allows only dashes to be used as separators in session entity label. "
"Underscores, spaces, slashes, and special characters (including #) are expressly forbidden."
"BIDS allows only the plus sign to be used as a separator in the subject entity label. "
"Underscores, dashes, spaces, slashes, and other special characters (including #) are "
"expressly forbidden."
),
solution="Rename the session without using spaces or underscores.",
solution="Rename the session without using any special characters except for `+`.",
examples=[
"`ses_01` -> `ses-01`",
"`session #2` -> `session-2`",
"`id 2 from 9/1/25` -> `id-2-9-1-25`",
"`ses_01` -> `ses+01`",
"`session #2` -> `session+2`",
"`id 2 from 9/1/25` -> `id+2+9+1+25`",
],
field="nwbfile.session_id",
source_file_paths=file_paths,
Expand Down
2 changes: 1 addition & 1 deletion src/nwb2bids/bids_models/_model_globals.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
_VALID_ID_REGEX = r"^[A-Za-z0-9]+"
_VALID_ID_REGEX = r"^[A-Za-z0-9+]+"
_VALID_SPECIES_REGEX = r"([A-Z][a-z]* [a-z]+)|(http://purl.obolibrary.org/obo/NCBITaxon_\d+)"
_VALID_BIDS_SEXES = {
value: True
Expand Down
13 changes: 7 additions & 6 deletions src/nwb2bids/bids_models/_participant.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,14 +100,15 @@ def _check_fields(self, file_paths: list[pathlib.Path] | list[pydantic.HttpUrl])
title="Invalid participant ID",
reason=(
"The participant ID contains invalid characters. "
"BIDS allows only dashes to be used as separators in subject entity label. "
"Underscores, spaces, slashes, and special characters (including #) are expressly forbidden."
"BIDS allows only the plus sign to be used as a separator in the subject entity label. "
"Underscores, dashes, spaces, slashes, and other special characters (including #) are "
"expressly forbidden."
),
solution="Rename the subject without using spaces or underscores.",
solution="Rename the subject without using any special characters except for `+`.",
examples=[
"`ab_01` -> `ab-01`",
"`subject #2` -> `subject-2`",
"`id 2 from 9/1/25` -> `id-2-9-1-25`",
"`ab_01` -> `ab+01`",
"`subject #2` -> `subject+2`",
"`id 2 from 9/1/25` -> `id+2+9+1+25`",
],
field="nwbfile.subject.subject_id",
source_file_paths=file_paths,
Expand Down
33 changes: 17 additions & 16 deletions tests/integration/test_messages.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,12 @@ def test_messages_1(problematic_nwbfile_path_1: pathlib.Path, temporary_bids_dir
nwb2bids.InspectionResult(
title="Invalid participant ID",
reason=(
"The participant ID contains invalid characters. BIDS allows only dashes to be used as separators in "
"subject entity label. Underscores, spaces, slashes, and special characters (including #) are "
"expressly forbidden."
"The participant ID contains invalid characters. BIDS allows only the plus sign to be used as a "
"separator in the subject entity label. Underscores, dashes, spaces, slashes, and other special "
"characters (including #) are expressly forbidden."
),
solution="Rename the subject without using spaces or underscores.",
examples=["`ab_01` -> `ab-01`", "`subject #2` -> `subject-2`", "`id 2 from 9/1/25` -> `id-2-9-1-25`"],
solution="Rename the subject without using any special characters except for `+`.",
examples=["`ab_01` -> `ab+01`", "`subject #2` -> `subject+2`", "`id 2 from 9/1/25` -> `id+2+9+1+25`"],
Comment on lines +41 to +46
Copy link
Collaborator Author

@CodyCBakerPhD CodyCBakerPhD Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A follow-up PR will add a common registry for notifications to avoid the need for this ctrl+r duplication (as discussed in #103)

field="nwbfile.subject.subject_id",
source_file_paths=nwb_paths,
data_standards=[nwb2bids.DataStandard.BIDS, nwb2bids.DataStandard.DANDI],
Expand Down Expand Up @@ -118,12 +118,12 @@ def test_messages_2(problematic_nwbfile_path_2: pathlib.Path, temporary_bids_dir
nwb2bids.InspectionResult(
title="Invalid participant ID",
reason=(
"The participant ID contains invalid characters. BIDS allows only dashes to be used as separators in "
"subject entity label. Underscores, spaces, slashes, and special characters (including #) are "
"expressly forbidden."
"The participant ID contains invalid characters. BIDS allows only the plus sign to be used as a "
"separator in the subject entity label. Underscores, dashes, spaces, slashes, and other special "
"characters (including #) are expressly forbidden."
),
solution="Rename the subject without using spaces or underscores.",
examples=["`ab_01` -> `ab-01`", "`subject #2` -> `subject-2`", "`id 2 from 9/1/25` -> `id-2-9-1-25`"],
solution="Rename the subject without using any special characters except for `+`.",
examples=["`ab_01` -> `ab+01`", "`subject #2` -> `subject+2`", "`id 2 from 9/1/25` -> `id+2+9+1+25`"],
field="nwbfile.subject.subject_id",
source_file_paths=nwb_paths,
target_file_paths=None,
Expand All @@ -135,14 +135,15 @@ def test_messages_2(problematic_nwbfile_path_2: pathlib.Path, temporary_bids_dir
title="Invalid session ID",
reason=(
"The session ID contains invalid characters. "
"BIDS allows only dashes to be used as separators in session entity label. "
"Underscores, spaces, slashes, and special characters (including #) are expressly forbidden."
"BIDS allows only the plus sign to be used as a separator in the subject entity label. "
"Underscores, dashes, spaces, slashes, and other special characters "
"(including #) are expressly forbidden."
),
solution="Rename the session without using spaces or underscores.",
solution="Rename the session without using any special characters except for `+`.",
examples=[
"`ses_01` -> `ses-01`",
"`session #2` -> `session-2`",
"`id 2 from 9/1/25` -> `id-2-9-1-25`",
"`ses_01` -> `ses+01`",
"`session #2` -> `session+2`",
"`id 2 from 9/1/25` -> `id+2+9+1+25`",
],
field="nwbfile.session_id",
source_file_paths=nwb_paths,
Expand Down
Loading