Skip to content

Rename MTS suffix #135

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Feb 16, 2023
Merged

Rename MTS suffix #135

merged 3 commits into from
Feb 16, 2023

Conversation

sandrinebedard
Copy link
Member

Description

This PR addresses #116 . It renames all MTS files to be BIDS compatible:

acq-MTon_MTS → flip-1_mt-on_MTS
acq-MToff_MTS → flip-1_mt-off_MTS
acq-T1w_MTS → flip-2_mt-off_MTS

@mguaypaq
Copy link
Member

The change itself looks good (that is, it looks like you correctly renamed all the files). The Dataset Validator check is failing, though. I re-ran it several times because of some transient issues (scikit-learn versus sklearn in some dependency, temporary network problem while downloading from amazon), but this run looks like it's an actual issue:

sg_params_checker -path-in ./
Traceback (most recent call last):
  File "/opt/hostedtoolcache/Python/3.7.15/x64/bin/sg_params_checker", line 11, in <module>
    load_entry_point('spinegeneric==2.5.dev0', 'console_scripts', 'sg_params_checker')()
  File "/opt/hostedtoolcache/Python/3.7.15/x64/lib/python3.7/site-packages/spinegeneric/cli/params_checker.py", line 86, in main
    MTS_acq = item.filename.split("_acq-")[1].split(".")[0]
IndexError: list index out of range

It looks like the params_checker is using the old filename format. I'll make a (backwards-compatible) fix upstream and re-run the Dataset Validator check for this PR after.

Copy link
Member

@mguaypaq mguaypaq left a comment

Choose a reason for hiding this comment

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

Once I can get the Dataset Validator check to pass, this looks like it can be merged.

@sandrinebedard
Copy link
Member Author

Is there any update on this problem? Thanks!

@mguaypaq
Copy link
Member

Sorry, I haven't had time to work on fixing the Dataset Validator. But:

  • I've looked into it enough to see that this is because of various bugs in the Dataset Validator, which will take some time to fix; and
  • Clearly just renaming the files should not affect the things that the Dataset Validator is supposed to be checking anyway.

So, I'll override the check and merge this change now, and open a separate issue for fixing the Dataset Validator.

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