Read the 2dseq subj_id from VisuSubjectId, not from the subject's name - #219
Merged
Merged
Conversation
Dataset.subj_id meant two different things depending on the dataset type. For fid, rawdata and traj it is SUBJECT_id, the subject identifier. For 2dseq it was VisuSubjectName, the subject's name, which ParaVision documents as a separate field (PV6 D02 2.4.11.4: VisuSubjectName is the name of the subject in DICOM format, VisuSubjectId the subject identification / registration string). The identifier exists in visu_pars as VisuSubjectId and was never read. On the public PV5.1 data (Zenodo 4048286, 0.2H2) the same scan gave subj_id == 'LEGO_PHANTOM' as a 2dseq and '0' as a fid. On PV360 the two diverge even when the user typed the same text in both fields, because VisuSubjectName is written in DICOM patient-name format: the 360 standard dataset gives 'std_PV360_3.6^^^^' for a 2dseq where VisuSubjectId is 'std_PV360_3.6' -- and, since id is composed from subj_id, carets inside a dataset identifier. Read VisuSubjectId, the Visu counterpart of SUBJECT_id (ParaVision derives one from the other), so subj_id means the same thing for every dataset type. The name stays available as metadata["visu_subject"]["name"]. This changes the 2dseq id string, which report() prints and which keys the property references: 2DSEQ_2_1_LEGO_PHANTOM_2 becomes 2DSEQ_2_1_0_2. The test/config references are renamed in place; no other property changes. study_id is left alone: all four recipes read the study *number* (VisuStudyNumber / SUBJECT_study_nr) where ParaVision's identifier is VisuStudyId / SUBJECT_study_name. That is consistent across types, so it is a naming decision for the maintainers rather than a defect. Closes isi-nmr#216 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NHhfFXMNZKMuPm3ppdzFXe
gdevenyi
added a commit
to gdevenyi/brukerapi-python
that referenced
this pull request
Aug 22, 2026
Every dataset type defined study_id as the study number -- str(VisuStudyNumber) for 2dseq, str(SUBJECT_study_nr) for fid, rawdata, traj and fid_proc -- while ParaVision's study identifier is a different parameter: VisuStudyId / SUBJECT_study_name, the user-given string set during study registration (PV6 D02 2.4.11.5). On the PV5.1 Zenodo data that is 'TEST_IO' against a number of 2. This was raised in isi-nmr#216 as a naming question; the answer is to follow the same pattern as subj_id. study_id now reads VisuStudyId / SUBJECT_study_name; the number moves to a new study_nr property, read as the integer it is. id keeps composing from the number -- it is what ParaVision keys the study directory by (the trailing _<session>_<study number> of its name, FILE_FORMAT.md 1.1), whereas a user-given identifier need not be unique -- so no dataset id, report or property reference changes. Neither property appears in to_dict()/report() output: study_id was already on the exclusion list and study_nr joins it, like the other id components. Builds on isi-nmr#219, which makes the 2dseq subj_id the subject identifier. Refs isi-nmr#216 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NHhfFXMNZKMuPm3ppdzFXe
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
2 participants
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.
Fixes #216.
Dataset.subj_idmeant two different things depending on the dataset type. Forfid,rawdataandtrajit isSUBJECT_id, the subject identifier. For2dseqit wasVisuSubjectName, the subject's name, which ParaVision documents as a separate field (PV6 D02 §2.4.11.4). The identifier exists invisu_parsasVisuSubjectIdand was never read.VisuSubjectIdis the Visu copy ofSUBJECT_id— in every study of the test corpus the two are equal, and the name is not:SUBJECT_idVisuSubjectIdVisuSubjectName0.2H200LEGO_PHANTOMlego_phantom_3lego_phantom_3lego_phantom_3LEGO_PHANTOMLEGO_PHANTOMLEGO_PHANTOMsubjectshipped)std_PV360_3.6std_PV360_3.6^^^^So on PV5.1 the same scan gave
subj_id == 'LEGO_PHANTOM'as a 2dseq and'0'as a fid, and on PV360 the 2dseqidcarried the DICOM patient-name carets (2DSEQ_T1_FLASH_1_std_PV360_3.6^^^^_1).Change
Read
VisuSubjectIdinproperties_2dseq_custom.json, sosubj_idmeans the same thing for every dataset type. The name stays available asmetadata["visu_subject"]["name"], so no separatesubj_nameproperty is added.This changes the 2dseq
idstring, whichreport()prints and which keystest/config/properties_*.json:2DSEQ_2_1_LEGO_PHANTOM_2becomes2DSEQ_2_1_0_2,…_std_PV360_3.6^^^^_1becomes…_std_PV360_3.6_1. The references were regenerated against the corpus and renamed in place — 35 PV5.1 and 19 PV360 entries, key andidonly, nothing else in any entry changed (the PV6/PV7 ids are unchanged because there name and id coincide). Probably worth a line in the release notes.Scope
study_idis left alone. All four recipes read the study number (VisuStudyNumber/SUBJECT_study_nr) where ParaVision's identifier isVisuStudyId/SUBJECT_study_name(TEST_IOvs2on the PV5.1 data). That is consistent across dataset types, so it is a naming decision for you rather than a defect; say the word and it can follow the same pattern (study_id← identifier,study_nr← number).Test
test_subj_id_is_the_subject_identifier_not_the_subject_name— a synthetic study whereVisuSubjectNameandVisuSubjectIddiffer; assertssubj_id == SUBJECT_idand the resultingid. Fails on master. The existingtest_metadata_reports_the_same_string_as_the_property_that_reads_itnow comparessubj_idagainst theidfield it reads.Suite: 2251 passed, 4 skipped.
🤖 Generated with Claude Code
https://claude.ai/code/session_01NHhfFXMNZKMuPm3ppdzFXe