FIX: dipole 3d plotting example shows dipole 6 as best dipole, however it's dipole 7 #4431
Conversation
|
|
||
| plt.suptitle('Dipole %s, Time: %.3fs, GOF: %.1f, Amplitude: %.1fnAm\n' % ( | ||
| idx, dipole.times[idx], dipole.gof[idx], dipole.amplitude[idx] * 1e9) + | ||
| idx + 1, dipole.times[idx], dipole.gof[idx], dipole.amplitude[idx] * 1e9) + |
There was a problem hiding this comment.
we could alternatively write idx or index in the title.
There was a problem hiding this comment.
@agramfort @Eric89GXL another option would be to use # as in ICA which seems to indicate index semantics in our plots (E.g. ICA #000)
|
no strong feeling if it's readable.
Python user should be aware of zero indexing and I think we are pretty
consistent with this.
|
I'm not seeing any consistence here. Dipole 6 is not Dipole 7 it's Dipole 6. If it is the index let's call it like this. Otherwise it makes you look up things at index 5. |
|
or write Dipole[6] ?
|
|
Yes |
Codecov Report
@@ Coverage Diff @@
## master #4431 +/- ##
=========================================
Coverage ? 80.28%
=========================================
Files ? 349
Lines ? 64188
Branches ? 9879
=========================================
Hits ? 51534
Misses ? 9863
Partials ? 2791 |
|
I don't have a strong opinion - |
|
we could also write: Dipole 1 / 7 |
|
I like the last proposal. But Dipole[6] would be ok too.
…On Fri, Jul 28, 2017 at 4:48 PM Alexandre Gramfort ***@***.***> wrote:
we could also write:
Dipole 1 / 7
Dipole 6 / 7
etc.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#4431 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AB0fiiNE6qmo1JV8eOJmJ_9Nyb4zXk48ks5sSfTFgaJpZM4OkesB>
.
|
|
Pick your choice and let's merge
…On 28 Jul 2017, 16:52 +0200, Denis A. Engemann ***@***.***>, wrote:
I like the last proposal. But Dipole[6] would be ok too.
On Fri, Jul 28, 2017 at 4:48 PM Alexandre Gramfort ***@***.***>
wrote:
> we could also write:
>
> Dipole 1 / 7
> Dipole 6 / 7
> etc.
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <#4431 (comment)>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AB0fiiNE6qmo1JV8eOJmJ_9Nyb4zXk48ks5sSfTFgaJpZM4OkesB>
> .
>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
@dengemann do you have time for this in the next week or so to make it into the next release? |
|
Will do later. Should take less then a minute.
…On Fri, 11 Aug 2017 at 03:53, Eric Larson ***@***.***> wrote:
@dengemann <https://github.com/dengemann> do you have time for this in
the next week or so to make it into the next release?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#4431 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AB0fispieGU4mtOfHGae6gKp5iF078XMks5sW7QzgaJpZM4OkesB>
.
|
|
@dengemann can you get to this in the next few days or should someone else take over? |
|
@dengemann can you get to this in the next few days or should someone else take over? It's getting close to 0.15 release time |
|
Will takeover tomorrow unless I hear otherwise |
|
Sorry, was not available the last days. I kept away from this as I wanted to see how the ICA component display got resolved. I'd suggest to just copy the pattern from there. |
|
There is no equivalent pattern for ICA, as those are channel names (and are now referred to that way as often as possible IIRC). |
5 participants
@agramfort @Eric89GXL do you agree that this is not ideal / confusing and should be adjusted?