Skip to content

Make dry run tests more extensive#46

Merged
Dhananjhay merged 8 commits intomainfrom
djay/fix-dry-run-tests
Apr 9, 2025
Merged

Make dry run tests more extensive#46
Dhananjhay merged 8 commits intomainfrom
djay/fix-dry-run-tests

Conversation

@Dhananjhay
Copy link
Copy Markdown
Contributor

This PR addresses issue #42.

@Dhananjhay
Copy link
Copy Markdown
Contributor Author

The dry run tests in the CI pipeline have been expanded. Previously, we only ran a basic version of the pipeline using a dummy T1w BIDS dataset. Now, I have added the following tests:

- name: Test T1w modality
- name: Test T2w modality
- name: Test CT modality
- name: Test stereotaxy feature with T1w modality
- name: Test stereotaxy feature with T2w modality
- name: Test fidqc feature

While setting up these tests, I also discovered a minor bug in the execution of the autoafids pipeline for the CT modality. In snakebids.yml, the key name was capitalized as CT, whereas the --modality flag allowed users to pass ct.

Additionally, if we decide to use Conda, the tests will be expanded to include Python versions 3.11 and 3.12.
@ataha24 @mackenziesnyder

@mackenziesnyder
Copy link
Copy Markdown
Contributor

The dry run tests in the CI pipeline have been expanded. Previously, we only ran a basic version of the pipeline using a dummy T1w BIDS dataset. Now, I have added the following tests:

- name: Test T1w modality
- name: Test T2w modality
- name: Test CT modality
- name: Test stereotaxy feature with T1w modality
- name: Test stereotaxy feature with T2w modality
- name: Test fidqc feature

While setting up these tests, I also discovered a minor bug in the execution of the autoafids pipeline for the CT modality. In snakebids.yml, the key name was capitalized as CT, whereas the --modality flag allowed users to pass ct.

Additionally, if we decide to use Conda, the tests will be expanded to include Python versions 3.11 and 3.12. @ataha24 @mackenziesnyder

Awesome looks good! I will test it out!


- name: Setup env for autoafids
run: |
echo "AUTOAFIDS_CACHE_DIR=`pwd`/test_data/autoafids_cache_dir" >> $GITHUB_ENV
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

for the AUTOAFIDS_CACHE_DIR, I think if we combine the dry and wet runs it will be overwritten for the model cache dir in the wet run. Will this be alright or was this section added to cache the setup environment
(dependencies etc.)?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I removed this from my code - not longer a conflict!

@Dhananjhay Dhananjhay merged commit 08c0745 into main Apr 9, 2025
3 checks passed
@Dhananjhay Dhananjhay deleted the djay/fix-dry-run-tests branch April 9, 2025 12:55
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