Skip to content

feat: read sessions.tsv files + pytest - #46

Open
WillForan wants to merge 3 commits into
bids-standard:masterfrom
WillForan:sessions.tsv
Open

feat: read sessions.tsv files + pytest#46
WillForan wants to merge 3 commits into
bids-standard:masterfrom
WillForan:sessions.tsv

Conversation

@WillForan

@WillForan WillForan commented Jul 11, 2025

Copy link
Copy Markdown
Contributor

For issue #35 - sessions.tsv can handle age changes across session

Sorry for the large diff. I wanted tests to confirm I didn't add obvious regressions. Doing so included some light refactoring. Would seem reasonable to reject for that reason alone -- too much changed.

This commit creates

  • read_participant_info to read in all session.tsv files and merge with participants.tsv

For testing

  • many minimal nii.gz (3dUndump) and .json files ({} and {"TaskName":..})
  • participant.tsv, session.tsv and scans.tsv examples/

Also for testing, some of run was refactored into dedicated functions

  • reading scan file (read_scan_date)

  • parse_args argument parsing

  • __init__.py imports main for easy pytest

    python3 -m pytest tests/

The session reading strategy is to merge participants.tsv and any sessions.tsv files.
sex and age can come from either but session.tsv columns are preferred for all sessions if any column exists in any session.tsv.

The scans.tsv file can now be omitted if acq_time column is in sessions.tsv but acq_time matches in scans.tsv will be used over sessoin.tsv when the former file exists.

work in progress! For issue bids-standard#35
Still need to test everything together for ses and non-ses versions

Strategy is to merge participants.tsv and any sessions.tsv
sex and age can come from either
but prefers session.tsv (for all sessions if exists for any)

can omit scans.tsv file if acq_time column is in sessions
but will use scans.tsv regardless when it exists

Created many empty nii.gz and .json files for testing.
Refactored reading scan file (`read_scan_date`) and created a few new functions for testing
and __init__.py imports main for easy pytest

   python3 -m pytest tests/

Makefile documents call, only runs on change; 'make test' writes .test
@WillForan
WillForan force-pushed the sessions.tsv branch 2 times, most recently from eb2dcfb to 43d5954 Compare July 11, 2025 18:53
Moved arg parsing and csv write out of run
Still writes metadata zip files inside run

examples/bids-* files now non-empty
 * '{}' for most json, '{"TaskName": "rest"}' for task-rest
 *  AFNI 3dUndump to create minimal nifti files
 * fix example tsv errors
   - sub-a_ses-1_scans.tsv gets session in filename column
   - sub-b_scans.tsv  references sub-b instead of sub-a (prev unmodded copy)

Initial tests for `run()`. Formatted with black.
Confirm `ses` and `noses` pulls `acq_times` from scans.tsv or sessions.tsv
For ses, scans.tsv can overwrite session.tsv
@WillForan WillForan changed the title feat: read sessions.tsv files + pytest [WIP] feat: read sessions.tsv files + pytest Jul 11, 2025
@yarikoptic

Copy link
Copy Markdown
Collaborator

this seems to be like a dump also of examples, some scripts there etc... could this may be shrunk to specifically what was desribed? (get rid of examples?)

@WillForan

Copy link
Copy Markdown
Contributor Author

Thanks for looking! I'm also uncomfortable with how large the change set is.

Tests for the new session parsing code (tests/test_session.py) currently use examples/ to confirm scans.tsv overwrites sessions.tsv and each session gets it's own date. But, somewhat redundantly, ‎examples/bids-ses_setup.bash is also tracked and annotates how the minimal nifti and json files are created. [None of the tabular files (examples/guid_map.txt, examples/**/*_sessions.tsv, nor examples/**/participants.tsv) are yet described by a script.]

Alternatives:

  1. Drop the filesystem specific tests, remove examples/

  2. remove examples/ from the repo. create on-the-fly when needed

    • as a Makefile recipe for make test?
      As it is, that'd require AFNI's 3dUndump to be available wherever the test is run.
    • rewrite bash script to python with nibabel, for easier depend management? could run as a pytest setup hook.
  3. zip up the examples and download if/when needed

    • as a separate bids2nda.data package?
    • stored somewhere else and curl in? (like this the least, seems easy to slip in something sketchy)

Case for keeping:
Minimally viable examples are useful for documentation too. The guid_map.txt file especially is novel to NDA.

session.tsv and scans.tsv might be outside the scope and better as links to bids-examples. On the other hand, all the bids-example/*/session.tsv files with age in the header have many more extraneous columns and none have both acq_time and age like examples/bids-ses/sub-b/sub-b_sessions.tsv

session_id	acq_time	age
ses-1	2010-12-31	15
ses-2	2015-01-01	19

@yarikoptic

Copy link
Copy Markdown
Collaborator

I lean somewhat towards 2. . I do love test fixtures and we create a lot across projects as e.g. in datalad @with_tree, and in dandi-cli starting entire server and populating records there... but I also see value from 'human observable' examples like bids-examples. There I do script https://github.com/bids-standard/bids-examples/pull/451/changes#diff-e24297ce70dde24311a763f6c08816cd5ca82acb2aae135de0359256da748d18 it.

Here, fixtures would allow for flexible configuration of various setups to try against, so hardcoding to one specific IMHO would be inefficient... since python, indeed creating some fixture and generate on the fly likely would be best for now (I bet claude or alike could RF for that), and later potentially with #50 test on specific "concrete" examples

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