Skip to content

Support alphanumeric sub or ses labels.#575

Merged
JoeZiminski merged 32 commits into
mainfrom
allow_alphanumeric_sub_ses_values
Sep 16, 2025
Merged

Support alphanumeric sub or ses labels.#575
JoeZiminski merged 32 commits into
mainfrom
allow_alphanumeric_sub_ses_values

Conversation

@JoeZiminski

@JoeZiminski JoeZiminski commented Aug 6, 2025

Copy link
Copy Markdown
Member

On the NeuroBlueprint side, alphanumeric characters are now allowed for sub- and ses- key labels. In datashuttle validation (that occurs when validating a project, or folder creation) the requirement is for labels to be numeric. This also comes with additional checks:

  • checking for duplicate sub/ses labels is extended to consider the integer value, rather than string comparison. For example "sub-001" and "sub-01" are considered equal.
  • numeric characters must all be the same length, zero padded if necessary (e.g. sub-01 and sub-002.

The former is not relevant for alphanumeric strings, only a direct comparison between the label string values is performed. Secondly, as per discussion, alphanumeric labels are designed to allow flexibility so there is no hard rule that mandates they must be the same length.

To handle this in datashuttle, the approach here is to introduce a new argument to validation (for project validation, validation from path and create_folders) to allow alphanumeric labels. If set, then labels such as sub-abc are allowed, duplicate sub/ses checks are on the string value, and labels must not all be the same length. Essentially this means that the packages behaves in the same way as previous, but now there is an option to add a little more flexibility. Checkboxes have been added to the TUI as required.

It is not ideal that allowing alphanumeric characters implicitly relaxes other checks (e.g. that labels no longer need to be the same length). Eventually, the solution will be to have very flexible rule-sets that can be used when validating (e.g. allow alphanumeric and having equal label lengths are separate rules). However, this will add a lot of arguments to validation and create folder function signatures, and its not clear how many rules we will have and how best to represent this in the software. For now, its easier just to allow the one argument for added flexibility for those who require it and formalize this properly down the line.

This PR tests:

  • the new entry to the tui persistent settings
  • validation for alphanumeric keys acts as expected
  • the new checkboxes in the TUI are set correctly and pass through to the underlying datashuttle functions correctly.

and adds these new options to the documentation (see the docs for this PR at this link).

@JoeZiminski JoeZiminski force-pushed the allow_alphanumeric_sub_ses_values branch from a0e06e5 to e82d675 Compare September 10, 2025 17:34

@cs7-shrey cs7-shrey left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM. Just a few suggestions.

Comment thread datashuttle/tui/interface.py Outdated
Comment thread datashuttle/utils/data_transfer.py
Comment thread tests/tests_tui/test_tui_widgets_and_defaults.py Outdated
Comment thread tests/tests_integration/test_validation.py Outdated
Comment thread tests/tests_transfers/local_filesystem/test_transfer.py Outdated
@JoeZiminski

JoeZiminski commented Sep 15, 2025

Copy link
Copy Markdown
Member Author

Thanks @cs7-shrey! I have applied the suggestions. I also renamed allow_alphanumeric_sub_ses_values to allow_letters_in_sub_ses_values further to a past meeting.

Will keep an eye on the tests, assuming they all pass this is ready I think.

@JoeZiminski JoeZiminski merged commit aef36f9 into main Sep 16, 2025
20 checks passed
@JoeZiminski JoeZiminski deleted the allow_alphanumeric_sub_ses_values branch September 16, 2025 11:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants