Skip to content

[MRG] ENH: Set initial time in Brain.add_data() #158

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

Merged
merged 7 commits into from
Jul 7, 2016

Conversation

christianbrodbeck
Copy link
Collaborator

Following up on mne-tools/mne-python#3375 CC @jaeilepp @Eric89GXL

@coveralls
Copy link

coveralls commented Jul 6, 2016

Coverage Status

Coverage decreased (-0.03%) to 76.157% when pulling 893e056 on christianbrodbeck:add_data into 75daf55 on nipy:master.

@agramfort
Copy link
Contributor

LGTM

+1 for merge if you're done.

@christianbrodbeck
Copy link
Collaborator Author

Updated the example. If others agree with the API then this is ready.

@coveralls
Copy link

coveralls commented Jul 6, 2016

Coverage Status

Coverage decreased (-0.03%) to 76.157% when pulling 6445a2b on christianbrodbeck:add_data into 75daf55 on nipy:master.

@agramfort
Copy link
Contributor

LGTM

@@ -857,6 +858,8 @@ def add_data(self, array, min=None, max=None, thresh=None,
conserving memory when displaying different data in a loop.
time_label_size : int
Font size of the time label (default 14)
initial_time : float
Copy link
Contributor

Choose a reason for hiding this comment

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

| None
and describe it means the first time point will be used

@larsoner
Copy link
Contributor

larsoner commented Jul 6, 2016

Otherwise LGTM, but you should probably also add a test that the parameter at least can be passed in

@coveralls
Copy link

coveralls commented Jul 6, 2016

Coverage Status

Coverage increased (+0.8%) to 77.02% when pulling c36d445 on christianbrodbeck:add_data into 75daf55 on nipy:master.

@coveralls
Copy link

coveralls commented Jul 6, 2016

Coverage Status

Coverage increased (+0.8%) to 77.02% when pulling c36d445 on christianbrodbeck:add_data into 75daf55 on nipy:master.

@coveralls
Copy link

coveralls commented Jul 6, 2016

Coverage Status

Coverage increased (+0.8%) to 77.02% when pulling c36d445 on christianbrodbeck:add_data into 75daf55 on nipy:master.

@christianbrodbeck
Copy link
Collaborator Author

Ready and tests pass, added test and doc.

@christianbrodbeck christianbrodbeck changed the title ENH: Set initial time in Brain.add_data() [MRG] ENH: Set initial time in Brain.add_data() Jul 6, 2016
stc['tmin'] + data.shape[1] * stc['tstep'],
data.shape[1])
time = np.linspace(stc['tmin'], stc['tmin'] + data.shape[1] * stc['tstep'],
data.shape[1])
Copy link
Contributor

Choose a reason for hiding this comment

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

The endpoint needs to be stc['tmin'] + (data.shape[1] - 1) * stc['tstep'] right?

@larsoner
Copy link
Contributor

larsoner commented Jul 6, 2016

Otherwise LGTM

@coveralls
Copy link

coveralls commented Jul 6, 2016

Coverage Status

Coverage increased (+0.8%) to 77.02% when pulling d12d48d on christianbrodbeck:add_data into 75daf55 on nipy:master.

@christianbrodbeck
Copy link
Collaborator Author

Good catch @Eric89GXL

@larsoner larsoner merged commit d6350fd into nipy:master Jul 7, 2016
@larsoner
Copy link
Contributor

larsoner commented Jul 7, 2016

Thanks @christianbrodbeck

@christianbrodbeck christianbrodbeck deleted the add_data branch February 9, 2017 16:11
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.

4 participants