Skip to content

Conversation

@jaeilepp
Copy link
Contributor

Though it turned out not to be the mess I expected, I decided to make a separate PR for this. This here would also be a good opportunity to design the layout so that it looks good. Now the layout uses pixels instead of inches so in theory it should work on all screen sizes and platforms.
Should there be a 'help' button somewhere that would open a pop up window to view all the key combinations etc. (discussed in #2206)? Are there other buttons we might need?

@agramfort
Copy link
Member

start with help button. and please share screenshots.

@jaeilepp
Copy link
Contributor Author

The point of the remake was to make it so that it looks the same on all platforms. The actual look of the layout shouldn't have changed much (apart from the title being larger). So the idea to make this a separate PR was to test this on all platforms and see if it turns out ok. I can test this on virtual box on my laptop (which has really weird screen settings) once I get home.
screenshot from 2015-06-18 13 57 46

@agramfort
Copy link
Member

agramfort commented Jun 18, 2015 via email

@jaeilepp jaeilepp changed the title Layout remake WIP Layout remake Jun 18, 2015
@mainakjas
Copy link
Contributor

by the way, one small issue I noticed with the raw plotter. If I select the bad channel, then the channel name doesn't get grayed out so it's a bit hard to say which channel got selected. @jaeilepp can you squeeze this into one of your PRs or make a new PR to address this. Should be an easy fix ...

@larsoner
Copy link
Member

One problem is that having a non-standard DPI will break the rendering:

image

You can set the figure.dpi param in matplotlibrc if you want to try it yourself, or do the equivalent in Python space with rcParams or whatever it is called. Not sure if there is an easy fix. Not the end of the world if there isn't.

For me the "+", "-", and "b" keys don't work, even though pageup/pagedown/home/end do. This makes sense given that they aren't listed in _plot_onkey in mne/viz/epochs.py, are you still working on adding those?

@larsoner
Copy link
Member

Also, 'escape' should close the epochs browsing window like it does for raw, and it should be added to the help.

Copy link
Member

Choose a reason for hiding this comment

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

indent pb

@agramfort
Copy link
Member

you'll need to rebase too.

@jaeilepp jaeilepp force-pushed the layout-remake branch 2 times, most recently from 20d97cc to 36e8ec7 Compare June 21, 2015 11:26
@jaeilepp
Copy link
Contributor Author

I guess it's best to finish the butterfly and continue with this when it's merged.

@agramfort
Copy link
Member

agramfort commented Jun 21, 2015 via email

@jaeilepp
Copy link
Contributor Author

Update:
Added a dialog for adjusting the number of epochs/channels per view as suggested in #2213.
screenshot from 2015-06-24 15 04 04

@teonbrooks
Copy link
Member

this was missed in the last pr: could you a command line message to remind/notify the user of the drop channel similar to the drop epochs message. would be very useful.

@jaeilepp
Copy link
Contributor Author

Well the channels aren't actually dropped. They're just added to the bads list like in raw.plot.

@teonbrooks
Copy link
Member

@mainakjas, @yousrabk, and I were discussing having an interactive way of selecting channels. In my system acquisition software we have plot of the channel montage and you can select a set of sensors this way. we could also include some buttons for pre-defined ROIs, like the ones given in mne_browse_raw (temporal, occipital quadrants, etc.)

here's a screen capture of the window:

montage_view

@teonbrooks
Copy link
Member

sorry, yeah, a message to notify which channels are marked in info['bads']

@jaeilepp
Copy link
Contributor Author

There is already an 'order' param in plot_raw that allows the user to define the order of the shown channels. Don't know if it's used much.

@mainakjas
Copy link
Contributor

In the help, can you align all the colons ? And color the keyboard shortcut red / yellow or make it bold. See whatever looks good. Just some aesthetics to make it look professional :)

@mainakjas
Copy link
Contributor

More help dialog related comments:

@mainakjas
Copy link
Contributor

Some comments about the viewport dialog box

  • the title of the dialog box is not inclusive of the check boxes
  • the checkboxes update immediately in the plot but the viewport dimensions don't update until you press the update button. Can you make it consistent? Actually, it would be better to have no update button if possible.
  • T0 visible => zeroline
  • Y labels visible => channel names

Should we also have a checkbox for event-id and epoch-id for consistency?

@mainakjas
Copy link
Contributor

In the help dialog, can you right justify the bold stuff (like here for example: https://inkscape.org/en/doc/keys046.html). Also can we change the sentences at the bottom to look like the table of keyboard shortcuts? Something like:

click epoch -> mark bad epoch
click channel -> mark bad channel
right click -> vertical line at a time instant
middle click -> show channel name (butterfly plot)

Maybe put a small heading which says mouse controls or something like that?

@agramfort
Copy link
Member

Screen shot ?

@jaeilepp
Copy link
Contributor Author

the checkboxes update immediately in the plot but the viewport dimensions don't update until you press the update button. Can you make it consistent? Actually, it would be better to have no update button if possible.

This is a usability thing. If you change the viewport dimensions, it takes a while to update, which is not the case with the labels and vertical lines. Imagine trying to adjust the viewport to show every epoch and channel and not hitting the end of the slider immediately.

@jaeilepp
Copy link
Contributor Author

screenshot from 2015-06-26 12 09 04

@jaeilepp
Copy link
Contributor Author

jaeilepp commented Jul 8, 2015

Done.

Copy link
Member

Choose a reason for hiding this comment

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

:) :)

@agramfort
Copy link
Member

clean up and I'll merge.

@jaeilepp
Copy link
Contributor Author

jaeilepp commented Jul 8, 2015

Oops. Done.

Copy link
Member

Choose a reason for hiding this comment

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

missing empty line

@agramfort
Copy link
Member

Tell me travis is happy. I'll merge.

@jaeilepp
Copy link
Contributor Author

jaeilepp commented Jul 8, 2015

Oops again. There was an empty line at the end it just doesn't show it here and I didn't notice it either.

@jaeilepp
Copy link
Contributor Author

jaeilepp commented Jul 8, 2015

Okay, travis is happy.

agramfort added a commit that referenced this pull request Jul 8, 2015
@agramfort agramfort merged commit 09aa9c9 into mne-tools:master Jul 8, 2015
@agramfort
Copy link
Member

Thanks !

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.

6 participants