8000 WIP: Neuroscout related changes by adelavega · Pull Request #1 · effigies/fitlins · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

WIP: Neuroscout related changes #1

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Dec 3, 2018
Merged

WIP: Neuroscout related changes #1

merged 4 commits into from
Dec 3, 2018

Conversation

adelavega
Copy link
@adelavega adelavega commented Nov 30, 2018 •
edited
Loading
  • ModelSpecLoader is not working working when the model was specified as a string, rather than extract from the filesystem using BIDSLayout. I fix that here
    Not sure this will work w/ auto_model. Doesn't that return a dictionary (not a path, or json in string format)?
  • I need to be able to specify more than a single derivatives dir (neuroscout event files, fmriprep files) Changing fitlins to allow multiple derivatives, as in BIDSLayout

@adelavega adelavega changed the title Extract model paths from BIDSFile objects WIP: Neuroscout related changes Nov 30, 2018
@adelavega adelavega mentioned this pull request Nov 30, 2018
Merged
2 tasks
@adelavega
Copy link
Author
adelavega commented Dec 1, 2018 •
edited
Loading

@effigies Let me know what you think about specifying arbitrary derivatives directories. They way I did it here is that you can keep passing derivatives to be indexed by pybids.

E.g. fitlins [other args] --derivatives = /my/fmriprep --derivatives= /my/events --derivatives = /maybe/something else
Those argument are put together into a list that is passed to BIDSLayout. If none is specified, the default is True, and all derivatives in bids_dir/derivatives are indexed. I suppose we can also allow None if the user doesn't want any derivatives to be indexed (although when would that be the case)?

Also, it would be syntactically nicer if we didn't have to repeated the --derivatives argument and just pass a list, but Im not sure how to do that w/ argparse.

effigies
effigies reviewed Dec 3, 2018
View reviewed changes
Copy link
Owner
@effigies effigies left a comment

Choose a reason for hiding this comment

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

LGTM. One comment on the argument parser.

@effigies @adelavega
Store derivatives into a single list
8079d95 
Co-Authored-By: adelavega <aleph4@gmail.com>
@adelavega
Copy link
Author

If you want go ahead and merge and I can open up a new PR for any forthcoming changes... or I can keep going here up to you.

@effigies effigies merged commit 39f912f into effigies:mnt/dependency_updates Dec 3, 2018
@adelavega adelavega deleted the fix/modelstring branch December 3, 2018 18:33
effigies pushed a commit that referenced this pull request Mar 6, 2019
@kfinc
Merge pull request #1 from rciric/nistats_smoothing
2f95cfb 
Sketch implementation of smoothing kernel in nistats
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@effigies effigies effigies left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@adelavega @effigies
0