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

MNT: Pybids 0.7 compatibility #84

Merged
merged 40 commits into from
Dec 12, 2018

Conversation

effigies
Copy link
Collaborator

This branch tracks restoration of functionality to FitLins, using derivatives from fMRIPrep 1.2.1 and models updated according to the draft BIDS model spec.

The ds114 model will be updated as needed in bids-standard/bids-examples#130.

Needed modifications to fMRIPrep derivatives are being tracked in nipreps/fmriprep#1384.

@effigies
Copy link
Collaborator Author

Currently stalled out at bids-standard/pybids#284.

effigies and others added 26 commits November 26, 2018 16:25
WIP: Neuroscout related changes
@adelavega
Copy link
Collaborator

Summary of conversation with Tal:

  1. At higher-levels, it looks like to compute contrasts, you feed them into the design matrix as the "intercept" and then computer a one sample t-test on the results. Need to test that this works with a two sample t-test (represented as 1/-1 in BIDS StatsModels)
  2. We need to add support for flexible # of higher-levels. Currently only a "fixed" group level is supported. This is necessary for models with more than 1 run per subject (so that estimates are averages for each subject before passing on to group level)
  3. We could potentially use the same interface for 1st and higher levels. Need to think about a refactor down the line for this.

@effigies
Copy link
Collaborator Author

@adelavega Given that we can now run simple second-level models, should we green-light a pybids 0.7 release?

@adelavega
Copy link
Collaborator

Ummm... I think the functionality is all there, but are the docs up to date? It's a pretty big API change so that's an important piece

@adelavega
Copy link
Collaborator

adelavega commented Dec 12, 2018

Also, maybe it's time to merge this PR as well, and we can start a new base PR for adding more than 2 levels (I've started working on it a bit)

@effigies
Copy link
Collaborator Author

Okay, let me give it a cleanup review.

@adelavega
Copy link
Collaborator

adelavega commented Dec 12, 2018

The main thing missing from previous functionality is some of the plotting-- design matrix correlation plot (no obvious "EVs"), and contrast matrix plotting. I think it would be reasonable to raise issues for restoring the function of those two (I don't personally need them so not a high priority for me)

@effigies
Copy link
Collaborator Author

There are also some weirdnesses with file naming. e.g.

derivatives/fitlins/sub-01/ses-retest/sub-01_ses-retest_task-fingerfootlips_bold_contrast-[array([[ 0. , 0. , 0. , 0. , 0. , 0. , 0. , 0. , 0. , 0. , 0. ,0. , 0. , 1. , -0.5, -0.5, 0. , 0. , 0. , 0. , 0. , 0. ,0. ]])]_ortho.png

Copy link
Collaborator Author

@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.

From a quick read through, some notes for myself.

fitlins/interfaces/nistats.py Outdated Show resolved Hide resolved
fitlins/interfaces/nistats.py Outdated Show resolved Hide resolved
requirements.txt Outdated Show resolved Hide resolved
requirements.txt Outdated Show resolved Hide resolved
@adelavega
Copy link
Collaborator

oh, I thought I fixed that. the issue is its using .format(contrast) instead of .format(contrast['name'])

@effigies
Copy link
Collaborator Author

Oh, I think you did. I still had some old outputs in my derivatives directory.

@effigies effigies merged commit ab13a6e into poldracklab:master Dec 12, 2018
@effigies effigies mentioned this pull request Dec 18, 2018
2 tasks
@effigies effigies deleted the mnt/dependency_updates branch February 1, 2019 14:36
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