Skip to content
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

PR1119 Review #1

Merged
merged 34 commits into from
May 3, 2024
Merged

PR1119 Review #1

merged 34 commits into from
May 3, 2024

Conversation

AliceJoubert
Copy link

@AliceJoubert AliceJoubert commented Apr 30, 2024

With this PR :

  • The recent modifications from dev are merged

The modifications allowing the code to run properly are inside clinica/iotools/converters/adni_to_bids/adni_utils.py :

  • A duplicated loop inside the function create_adni_sessions_dict was removed
  • The check on parameter mod_to_update inside the function create_file was also removed

Do not hesitate if you have questions or comments.

NicolasGensollen and others added 30 commits April 3, 2024 08:33
…eprocessing utils (aramis-lab#1116)

* Move DWI specific functions from filemanip utils to dwi utils preproc pipeline utils

* Fix typing that works only for Python >= 3.10

* Clean api
Bumps [pillow](https://github.com/python-pillow/Pillow) from 10.2.0 to 10.3.0.
- [Release notes](https://github.com/python-pillow/Pillow/releases)
- [Changelog](https://github.com/python-pillow/Pillow/blob/main/CHANGES.rst)
- [Commits](python-pillow/Pillow@10.2.0...10.3.0)

---
updated-dependencies:
- dependency-name: pillow
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* add definition for FWHM

* add link for FWHM
* add definition for SUVR in glossary and link in ML_prepareSVM description

* Update docs/glossary.md

---------

Co-authored-by: Gensollen <[email protected]>
…ramis-lab#1127)

* Check that BIDS folders have a dataset_description.json file

* make the _check_bids_is_not_empty function a bit more robust
)

* Use of regex for subject conversion to RID and modification of associated unit tests

* test lint
* Use an enum for the study names

* update unit tests

* forgot one
Bumps [idna](https://github.com/kjd/idna) from 3.6 to 3.7.
- [Release notes](https://github.com/kjd/idna/releases)
- [Changelog](https://github.com/kjd/idna/blob/master/HISTORY.rst)
- [Commits](kjd/idna@v3.6...v3.7)

---
updated-dependencies:
- dependency-name: idna
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* refactor freesurfer pipelines

* add forgotten cli

* use absolute imports in task definitions

* allow string or Path for get_subject_id

* fix folder creation in init_input_node
* run linter on maint branches

* run unit tests on maint branches

* run converter non regression tests on maint branches

* run instantiation tests on maint branches
* run linter on maint branches

* run unit tests on maint branches

* run converter non regression tests on maint branches

* run instantiation tests on maint branches
* update version number

* update changelog
* update OASIS website URL

* Apply suggestions from code review
aramis-lab#1159)

* Wrong Processed sequence --> Results: Impossible to make bids of FDG PET Uniform (does not find it)

* Linter exception

* Linter word-list exception update

* Update clinica/iotools/converters/adni_to_bids/adni_modalities/adni_fdg_pet.py

Co-authored-by: Gensollen <[email protected]>

* Update pyproject.toml

Co-authored-by: Gensollen <[email protected]>

* Update clinica/iotools/converters/adni_to_bids/adni_modalities/adni_fdg_pet.py

Co-authored-by: Gensollen <[email protected]>

* Update clinica/iotools/converters/adni_to_bids/adni_modalities/adni_fdg_pet.py

* try using a text file for ignoring words

* remove useless codespell:ignore

* sort ignore words

---------

Co-authored-by: Gensollen <[email protected]>
@AliceJoubert
Copy link
Author

Hello @tharpm95,

I have a small question regarding the function 'rename_maps' since I am not yet totally familiar with fmaps and corresponding dcm2nix outputs. Could you tell me what you are expecting at the output of dcm2nix in terms of files (json/nifti) and what you want to do with them ?

@tharpm95
Copy link

tharpm95 commented May 2, 2024

Hello @tharpm95,

I have a small question regarding the function 'rename_maps' since I am not yet totally familiar with fmaps and corresponding dcm2nix outputs. Could you tell me what you are expecting at the output of dcm2nix in terms of files (json/nifti) and what you want to do with them ?

Sure. First of all, thank-you for the PR, it has run successfully without issues. I will go ahead and merge the request unless you want to add more? Regarding the fmaps, there is a PDF which I have referenced which provides file naming convention information. I tried to follow BIDS specification version 1.1.1 section 8.3.5.1 Case 1 and Case 2. The file names listed in that specification are what is expected as outputs from dcm2niix. The reason that we want the fmaps is because we are processing resting state fMRI data for all ADNI participants at our center, and we would like to convert the data to BIDS before processing. However, without also having the field maps, we cannot correct for magnetic field distortions thus limiting our analyses. I hope that this answers your question.

@tharpm95 tharpm95 merged commit 2b34264 into IUSCA:clinica_pr_040324 May 3, 2024
@AliceJoubert
Copy link
Author

Hello @tharpm95,
I have a small question regarding the function 'rename_maps' since I am not yet totally familiar with fmaps and corresponding dcm2nix outputs. Could you tell me what you are expecting at the output of dcm2nix in terms of files (json/nifti) and what you want to do with them ?

Sure. First of all, thank-you for the PR, it has run successfully without issues. I will go ahead and merge the request unless you want to add more? Regarding the fmaps, there is a PDF which I have referenced which provides file naming convention information. I tried to follow BIDS specification version 1.1.1 section 8.3.5.1 Case 1 and Case 2. The file names listed in that specification are what is expected as outputs from dcm2niix. The reason that we want the fmaps is because we are processing resting state fMRI data for all ADNI participants at our center, and we would like to convert the data to BIDS before processing. However, without also having the field maps, we cannot correct for magnetic field distortions thus limiting our analyses. I hope that this answers your question.

Thanks for your answer, that's clearer ! I will be working on the PR you opened on Clinica after this so you can close my PR.

@AliceJoubert AliceJoubert deleted the fmap040324 branch May 7, 2024 08:40
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.

5 participants