Skip to content
This repository has been archived by the owner on Apr 2, 2020. It is now read-only.

various cosmits #39

Merged
merged 2 commits into from
Nov 27, 2015
Merged

various cosmits #39

merged 2 commits into from
Nov 27, 2015

Conversation

agramfort
Copy link
Contributor

result on my we procrastination...

@agramfort
Copy link
Contributor Author

just a remark but you certainly already know. the options for hrf_model parameter are not consistent in the package.

@agramfort agramfort closed this Nov 22, 2015
@agramfort agramfort reopened this Nov 22, 2015
@agramfort
Copy link
Contributor Author

I am not sure I understand the rationale for the check_design_matrix that is called in user code just to extract the numpy array from a design_matrix.

why not passing a design_matrix to FirstLevelGLM ?

I think that would simplify examples.

@bthirion
Copy link
Member

On 22/11/2015 09:12, Alexandre Gramfort wrote:

just a remark but you certainly already know. the options for
hrf_model parameter are not consistent in the package.


Reply to this email directly or view it on GitHub
#39 (comment).

Yes, please see #38...

@agramfort
Copy link
Contributor Author

great

@@ -6,6 +6,7 @@
"""

import os
import os.path as op
Copy link
Member

Choose a reason for hiding this comment

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

@GaelVaroquaux laughs at me when I do this, so I tend to avoid it now...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@GaelVaroquaux https://github.com/GaelVaroquaux laughs at me when I do
this, so I tend to avoid it now...

let's team up to laugh at him :)

Copy link
Contributor

Choose a reason for hiding this comment

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

I am not going to laugh at anyone but since you are using it once below, you may as well remove this import and use the more verbose os.path.

@bthirion
Copy link
Member

I'll have a closer look tomorrow, but this looks fine. Thanks a lot for taking time to improve nistats !

@bthirion
Copy link
Member

+1

In examples, I like to give people the concrete sense of where the
design matrix values are, but in terms of API, we should only use the
high-level objects. IIRC, this should work.

@agramfort
Copy link
Contributor Author

one idea I add also would be to use the index of a DataFrame rather than using the frame_times (don't like the name much btw)

In [4]: df = pd.DataFrame(dict(reg1=np.random.randn(10), reg2=np.random.randn(10)))

In [5]: df
Out[5]:
       reg1      reg2
0  0.435177 -1.034931
1 -0.059424  2.223872
2  0.070459 -0.838222
3 -0.777904 -0.690715
4 -0.782690 -0.045528
5  0.278254  0.388872
6  0.867945  0.242028
7 -2.214093  0.866546
8 -0.186639 -0.626605
9 -0.635547  1.209869

In [6]: tr = 2.4

In [7]: frame_times = np.arange(10) * 2.4

In [8]: df.index = frame_times

In [9]: df
Out[9]:
          reg1      reg2
0.0   0.435177 -1.034931
2.4  -0.059424  2.223872
4.8   0.070459 -0.838222
7.2  -0.777904 -0.690715
9.6  -0.782690 -0.045528
12.0  0.278254  0.388872
14.4  0.867945  0.242028
16.8 -2.214093  0.866546
19.2 -0.186639 -0.626605
21.6 -0.635547  1.209869

In [10]: matrix = df.values

In [11]: matrix
Out[11]:
array([[ 0.43517668, -1.03493107],
       [-0.05942358,  2.22387234],
       [ 0.07045882, -0.83822237],
       [-0.77790391, -0.69071475],
       [-0.78269037, -0.04552835],
       [ 0.27825373,  0.38887175],
       [ 0.8679447 ,  0.2420283 ],
       [-2.21409337,  0.8665459 ],
       [-0.18663918, -0.62660464],
       [-0.63554651,  1.20986888]])

@bthirion
Copy link
Member

Unless import os.path as op I agree with all changes.
Can anybody else review ?

@agramfort
Copy link
Contributor Author

agramfort commented Nov 24, 2015 via email

@bthirion
Copy link
Member

Let's say it's a matter of taste ;-) ... I'm not a purist...

paradigm = DataFrame({'name': conditions, 'onset': onsets})

# build design matrix
frametimes = np.linspace(0, (n_scans - 1) * tr, n_scans)
frame_times = np.linspace(0, (n_scans - 1) * tr, n_scans)
Copy link
Contributor

Choose a reason for hiding this comment

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

Use frame_times = np.arange(n_scans) * tr as above.

@lesteve
Copy link
Contributor

lesteve commented Nov 27, 2015

Other than the two minor comments, this looks fine to me.

@agramfort
Copy link
Contributor Author

comments addressed

@lesteve
Copy link
Contributor

lesteve commented Nov 27, 2015

Merging, thanks a lot!

lesteve added a commit that referenced this pull request Nov 27, 2015
@lesteve lesteve merged commit 2219074 into nilearn:master Nov 27, 2015
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants