Add an optional string field /nirs(i)/data(j)/name#178
Open
emiddell wants to merge 2 commits into
Open
Conversation
There was a problem hiding this comment.
Pull request overview
Adds an optional /nirs(i)/data(j)/name string field to the SNIRF specification to allow distinguishing multiple data(j) time series within a single /nirs(i) group when measurement-list metadata alone is insufficient (per #177).
Changes:
- Document
/nirs(i)/data(j)/nameas an optional, free-form string field in the SNIRF specification and add it to the table of contents. - Add an “in development” changelog entry noting the new
namefield.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| snirf_specification.md | Adds the new optional data(j)/name dataset definition and links it from the TOC. |
| CHANGELOG.md | Records the new field addition under the in-development section. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * **Type**: string | ||
| * **Location**: `/nirs(i)/data(j)/name` | ||
|
|
||
| This is an optional, free-form string describing the j<sup>th</sup> data time course. One use of this parameter is to distinguish processed time courses which are indistinguishable by `measurementList(k).dataType`, `measurementList(k).dataTypeLabel` and `measurementList(k).dataTypeIndex` (like for example changes in optical density which were filtered by different artifact-rejection methods). |
Comment on lines
+5
to
+7
| ### In development | ||
| * Add `nirs(i)/data(j)/measurementLists` as an alternative to the indexed-Group measurementList `nirs(i)/data(j)/measurementList(k)`. | ||
| * Add `nirs(i)/data(j)/name` as an optional, free-form string field to label time series (#177) |
| ### `2.0` In development | ||
| ### In development | ||
| * Add `nirs(i)/data(j)/measurementLists` as an alternative to the indexed-Group measurementList `nirs(i)/data(j)/measurementList(k)`. | ||
| * Add `nirs(i)/data(j)/name` as an optional, free-form string field to label time series (#177) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR contains the changes proposed in #177 .