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

Eg mod #338

Merged
merged 13 commits into from
Nov 30, 2020
Merged

Eg mod #338

merged 13 commits into from
Nov 30, 2020

Conversation

swaythe
Copy link
Contributor

@swaythe swaythe commented Nov 25, 2020

Removed the example file nipype_preproc_spm_nyu because 1) it errors, and 2) it uses the NYU rest fetcher which is deprecated. I will rework this example with a different dataset later because it showcases the use of an ini file.

The example coreg_demos uses the ABIDE dataset with hardcoded paths; removed it.

Added a line to the README to say that pypreprocess works with Ubuntu 16 and 18, no guarantees otherwise.

Copy link
Member

@bthirion bthirion left a comment

Choose a reason for hiding this comment

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

Almost there, thx !

@@ -30,12 +30,12 @@

# Path (relative or full) of directory containing data (if different from directory
# containing this configuration file).
dataset_dir =
dataset_dir =
Copy link
Member

Choose a reason for hiding this comment

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

Something wrong here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've removed the NYU rest ini file. The NYU rest example is obviously not the only one showcasing use of ini files. Was there a specific reason for including this dataset?

Copy link
Member

Choose a reason for hiding this comment

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

O that's an ini file, sorry.
Anyway, if the fetcher is deprecated, it's better to remove it (as you may have seen, Nilearn has examples on a new resting state dataset).
Best,
B

@bthirion
Copy link
Member

Maybe a rebase to remove conflicts ?
Best,
B

@swaythe
Copy link
Contributor Author

swaythe commented Nov 27, 2020

@bthirion The conflicts have been resolved, should I merge once the tests are complete?

@swaythe swaythe merged commit 53336b6 into neurospin:master Nov 30, 2020
@swaythe swaythe deleted the eg_mod branch November 30, 2020 13:38
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