Read study_id as the study identifier and expose the number as study_nr - #220
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
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
gdevenyi
force-pushed
the
fix/study-id-identifier
branch
from
August 22, 2026 02:29
f080d42 to
ba2bae7
Compare
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.
Follow-up to #216 (the
study_idquestion raised there) — stacked on #219, the first commit here is that PR's; merge #219 first.Every dataset type defined
study_idas the study number —str(#VisuStudyNumber)for 2dseq,str(#SUBJECT_study_nr)for fid/rawdata/traj/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 isTEST_IOagainst a number of2(data_io/2on PV6,API_TEST/1on PV7).Change
Same pattern as
subj_idin #219, in all fiveproperties_*_custom.json:(
#SUBJECT_study_name/#SUBJECT_study_nrfor the raw types.)study_nris the integer the parameter is, not a string.idkeeps composing from the number (@study_nr): it is what ParaVision keys the study directory by (the trailing_<session>_<study number>of its name,FILE_FORMAT.md§1.1 — note the number is unique within a session, so two sessions of one subject can repeat it, as before), whereas a user-given identifier need not be unique. So no dataset id, noreport()output and notest/configreference changes. Neither property shows up into_dict()/report():study_idwas already on the exclusion list andstudy_nrjoins it, like the otheridcomponents — un-hiding the identifier is a one-line follow-up if you want it reported.Test
test_study_id_is_the_study_identifier_and_study_nr_the_number— a synthetic study with a 2dseq and a fid,VisuStudyId/SUBJECT_study_name=TEST_IO, number 1; assertsstudy_id,study_nr, and that both ids are unchanged. Fails on master. (study_idis excluded from reports, so the corpustest_propertiescannot see a typo in the raw-type recipes — that is why the fid is in the test.)test_traj_custom_properties_define_a_stable_idupdated for the new key and theidrecipe.Suite: 2252 passed, 4 skipped.
🤖 Generated with Claude Code
https://claude.ai/code/session_01NHhfFXMNZKMuPm3ppdzFXe