Allow user-defined specification of vigilance states

[TomBugnon]

Hi all (@EtienneCmb, @raphaelvallat, and people following https://github.com/EtienneCmb/visbrain/issues/35 : @paulbrodersen, @grahamfindlay , @dougollerenshaw )

I finally implemented the long-awaited specification of arbitrary vigilance states from a (yaml) config file, which will I think will be quite useful to a lot of animal (and epilepsy) people.

How to use:

Run the exemple file at visbrain/examples/gui_sleep/load_edf_user_states_cfg.py , or:

1. Create a yaml file formatted as follows:

  state_1:  # Label of vigilance state
    value: 1  # Associated value on 'point-per-second'-style hypnogram. Mandatory key
    shortcut: 1  # Must require a single key-press. Mandatory key
    display_order: 1  # Position on GUI. Mandatory key
    color: 'red'  # matplotlib color, hexadecimal color or tuple of RGB. default is "black"
  state_2:   # Must be unique
    value: 2  # Must be unique
    shortcut: 2  # Must be unique
    display_order: 2.0  # Must be unique
    # color: 'black'  # 'black' if key is missing
  state_3: 
    value: 42
    shortcut: w
    display_order: 1.00005  # It's not the value but the rank across states that matters. 
    color: '#56bf8b'

(Note that the keys listed in the Shortcuts section are reserved and can't be used as shortcuts for scoring.)

2. Specify the path to you vigilance states configuration file when creating a Sleep instance with the states_config_file kwarg as follows:

from visbrain.gui import Sleep

Sleep(states_config_file='path/to/my/states_cfg_file.yml').show()

If not specified, Sleep uses the default (previously hardcoded) states, values, shortcuts and colors.

Here's an example with extra states:

Implementation and testing

• I basically added some attributes to the Sleep object (hstates, hvalues, hcolors, hshortcuts, hYranks, all lists of equal length) which are passed around as kwargs wherever needed. This is not of the utmost elegance but does the trick. semi-external functions such as write_fig_hypno use the hardcoded default when the kwargs are not specified.

• I added testing for hypnogram r/w and for states configuration reading

Changes in usage

• The trickiest part imo was to handle properly the saving and loading of hypnograms with different formats. I basically took the easiest path in terms of implementation and backwards compatibility: Regarding loading:

  • .edf and .hyp hypnograms may only be loaded or saved when using the default config for state/value mapping in Sleep.

  • One subtlety: Some .txt hypnograms in the examples (eg in examples/sleep/load_edf.py) use what I believe is EDF-style state/values mapping, with a hardcoded swapping of values in rw_hypno.py. I maintain this behavior and allow loading of this kind of .txt hypnogram only with the default config.

  • all other formats can be saved/loaded with any configuration. Loading fails if one of the states or values is absent from the config.

  • Note that I added an info popup (as well as raising a ValueError) in the failing scenarios stated above so that the user doesn't close Sleep thinking the hypnogram was saved properly.

• There's now an extra dependency (pyyaml)

• The href Sleep kwarg (which could be used to rearrange states on the GUI) is gone and replaced by the display_order key in the config yaml.

All other functionalities should work similarly but let me know if I missed anything

Other

• I didn't update HypnogramObj , which would require an entire rewriting. I'm not sure what the use for this class actually is. I can do it if needed.

• All Sleep-related tests pass on my machine. There are preexisting issues with continuous integration so I'm expecting the travis build to fail (at least) for independent reasons.

• @EtienneCmb I didn't check that the doc builds correctly, I might have made a couple formatting mistakes

• This probably deserves a change in version/release, let me know what you decide on that respect @raphaelvallat and @EtienneCmb . There's a few other (minor and major) changes I will implement next that would make for a great v1.0 I think

• Finally, the commit Fix min/max chan amps for all-positive(/neg) data should not be part of this PR and reiterates https://github.com/EtienneCmb/visbrain/pull/77 but it would be simpler for me to leave it in if you dn't mind

This is quite a big PR so thank you in advance for your time testing or reviewing this ! Looking forward to hearing your feedback Cheers, Tom

[grahamfindlay]

Just scored a couple files using this. Fantastic! Works really well and makes my life much easier. Some nice bonuses:

• Setting a state's value to 0 makes it the default state when Sleep launches without a hypnogram, so I can have a state representing unscored data as the default, instead of "Wake." Since "Wake" was a dangerous default, the first thing that I used to do when scoring a file was to open the "Scoring" tab and manually set the default to "Art." This fixes that.
• Setting my default state's color to "white" hides it visually, which I prefer.
• It's a small thing, but I do appreciate that I can make the hypnogram colors match the legends that I use everywhere else.
• Goes without saying, but having it be instantly backwards-compatible with all of my previously scored data (in stage-duration format, of course) is very convenient.

Thanks very much, Tom. And thanks for fixing #77 too.

[raphaelvallat]

Hi @TomBugnon,

This is really amazing. Thank you so much for all your hard work. As of now, Visbrain won't even launch on my Mac so I unfortunately cannot try the PR. But I trust that you've done a great job and I'm all in for merging this one and https://github.com/EtienneCmb/visbrain/pull/77 and then releasing a new version. The issue is that I don't have admin rights for this repo so I cannot merge the PR, and I don't know if @EtienneCmb still has time to work on Visbrain at all these days. I will drop him an email if he doesn't reply within the next week.

A few comments on the PR:

1. Why choose the YAML format to store the vigilance state instead of a JSON or txt file?
2. No worries for the HypnogramObj, I think we're never using it anyway (and should probably delete it)

Thanks again, Raphael

[TomBugnon]

Thanks for the feedback @raphaelvallat and @grahamfindlay !

1. Yaml is a personal preference for user-created config files. I find it easiest to write and more readable than json (which requires more lines and has some unintuitive requirements such as this and txt files (which don't support syntax highlighting and are unnatural to represent dictionaries). I don't mind switching to another format if you think this is not worth the extra dependency

I agree this and the previous "scoring window" PR are probably worth a release Just so you know, we are heavily using Sleep in our lab and I am basically tasked with making it our dream software for scoring so there's probably a couple more PRs upcoming:

• (soon) Fixing some minor bugs with display
• (soon) Manually selecting arbitrary epochs for scoring with mouse click-and-drag
• (eventually) somehow allowing scoring of data with synced video Happy to discuss those with anyone who might be interested,

Best

[raphaelvallat]

Sounds good for the YAML format. And thanks for the other PRs too, I will look at that now.

Unrelated, but since you're kind of the core maintainer of Sleep right now, I almost wonder if you should create an official Sleep fork? 1) You'd be able to push changes much faster, and 2) you could even remove the other modules of Visbrain to only keep a "lightweight" version with only Sleep... Curious to hear others' opinions on the above.

[paulbrodersen]

Curious to hear others' opinions on the above.

Other than having write access, is there a compelling reason for the fork? Forking from popular projects costs a lot of visibility and discoverability, and you loose the few helpful eyes that you have. Why not just give him write access to the repository?

[TomBugnon]

One sensible reason I can think of would be if Raphael and Etienne decide that they would rather maintain the original scope of Sleep (human scoring), for which the software was already doing a pretty good job, rather than extend and complexify the project to account for the needs of animal people. In that case it would make sense to have some sort of official fork dedicated to animal scoring with a different set of features (and possibly a focus on speed). I don't have a definite opinion about this at the moment.

[EtienneCmb]

Hi @TomBugnon,

This is a fantastic work, the code is really clean and there are already feedback of peoples that find it very useful. You really did a great job and I suppose that it was not always easy to overcome the verbosity of the code imposed by PyQt and Vispy.

I'm fine with the yml config files, its clear to read and also agree that the syntax highlighting make it nice, and I guess there are many ways to easily convert json to yml.

About your PR, I confess that I'm not really maintaining visbrain anymore, because I don't have the time for it. Therefore, I agree with Raphael that you should not be limited by our lack of investment. The big question is how to transfer the project. I understand the point of Raph. Visbrain also includes many other visualization modules. At the beginning, it was really useful as it also attracted people from the EEG/MEG/intracranial community with 3D plotting requirements. That said, it's also much harder to maintain.

As Paul said, there's a small growing community behind Visbrain (and Sleep) that you can potentially lose when starting from a fresh repo. At the same time, if you start from a fresh fork, you can also drastically simplify visbrain to have only the Sleep module and therefore, seasier maintenance. I don't have a solution for now but I'm totally open to any suggestion.

Probably the best solution (for now) is, as @paulbrodersen said, to give to @TomBugnon the write access (a second person would be great). What do you think @TomBugnon ? and @raphaelvallat ?

[raphaelvallat]

Hi all,

I agree that the best solution for the near future is to give @TomBugnon writing access! As Etienne said, the obvious advantage I see of creating a separate "fork" could be to keep only the Sleep module, which would facilitate maintenance. And from there one could potentially have two branches: one for human sleep scoring, the second for animal sleep. But just to be clear I don't think this is a perfect solution either, as it will undoubtedly be confusing for the users. For now, keeping everything in the official Visbrain repo and giving Tom writing access (+ adding him to the list of core contributors) is probably the best solution 👍

[TomBugnon]

Hi all, I'm fine with being given write access :) I don't know the other GUIs of visbrain and I'm not very comfortable with vispy/pyqt (atm) so it would be good to have more maintainers