Adds new waveform viewer

[Lucas-Mc]

This change adds a new waveform viewer which can be used alongside with LightWAVE. This is built using Plotly and Dash which provides flexible and interactive plotting which can easily be modified with just a few lines of code. It also employs the WFDB-Python package for easy interactions with records in Python which can later be extended for more uses within PhysioNet. My proposed way to access this viewer is through a "Demo" link shown at the top of the LightWAVE home page where input records are chosen (http://localhost:8000/lightwave/demobsn/1.0?db=demobsn/1.0 for example). Also, testing for this viewer can be done after #1176 is implemented though demobsn should work also but is minimal.

I compiled a list of Pros and Cons for this viewer as compared to LightWAVE for those deciding which viewer to use.

Pros:

• Can be used on the development and staging environment
• Can be built separately easily to help authors with reviewing waveforms (no sandboxed version required)
• Displays all errors encountered (at least it should unless I missed a case)
• Auto-scaling of signals to prevent impossible to read signals (no external calibrations files: wfdbcal)
• Can pick a subset of signals
• Values on y-axis are shown
• Interactive hover / pan / zoom
• Can read and plot (most) annotations without ANNOTATORS file (displays warning if no ANNOTATORS file)
• Displays all EKG / ECG signals first, then BP, then the rest in alphabetical order
• Some waveforms can not be read by LightWAVE / take a long time to load
• Easier to switch between records using Previous and Next button

Cons:

• No play feature for automatic scrolling
• Only shows a maximum of a minute at a time due to performance concerns
• Markedly slower typically only when annotations are enabled
• EDF is still a little sketchy (though almost fixed in WFDB-Python) DONE: Improves EDF speed / fixes bugs wfdb-python#274

At the end of the day, I know there's probably some undiscovered errors here (it has been tested on all available databases on PhysioNet which is extensive though not comprehensive) which can be amended later though I think this is a strong start and an improvement for PhysioNet.

[Lucas-Mc]

There is a conflict in the requirements for pytz which is being resolved here: MIT-LCP/wfdb-python#275

[Lucas-Mc]

This now should be rebased with the current dev branch so that the new demo project can be tested #1176

[tompollard]

The viewer looks good and it works nicely. The only real issue in terms of usability that I could see is that access/permissions need checking:

• currently, anyone (including anonymous users) can view waveforms for active (pre-publication) projects. Only admin and authors should be able to view at this stage. e.g. http://localhost:8000/waveforms/active_4riWnicqZRycqFZb6FUu/ can be viewed by an anonymous viewer
• are restricted/credentialed access restrictions applied, so that only authorised viewers can access the data?

The code could do with a clean up, especially physionet-django/waveforms/dash_apps/finished_apps/waveform_vis.py.

I have added comments inline, but I think the main things are:

• try to follow PEP8 (install a linter if you don't have one!)
• add docstrings
• if adding TODOs, make sure that someone new to the code would understand what needs to be done and why
• there are a fair amount of comments that don't really add to readability, so could be removed.
• breaking down the big functions would make the code much easier to follow!

[tompollard]

It would be good to try avoid all of these global variables. Perhaps they could be dropped into a config/settings function?

[briangow]

This looks great! Being able to select the signals you want to view is a nice feature. I had a few thoughts around potential improvements, none of which should be required for an initial release of this waveform viewer:

• When saving to a .png, it may be nice to add a small title indicating the database and record number.
• When zooming in on the time axis the xlabel ticks disappear eventually, perhaps consider creating new finer grain ticks when zooming in.
• Autoscale and reset axis seem to do the same thing, which sounds like the expected behavior if a custom axes range isn't set (https://plotly.com/chart-studio-help/getting-to-know-the-plotly-modebar/). Do you plan to make use of this custom axes option? If not, is it possible to remove the reset axis button?
• Waveforms with annotations show the annotation at the top of the plot but since there isn't a line or dot that connects to the waveform itself, you can't tell exactly which sample the annotation applies to. Consider adding a dot or line on the waveform to make this more precise.
• With 8 signals plotted the labels get very small and since they are also a gray color they become hard to read. Consider making the label pure black, or bold. You could also make the x-axis / time labels larger than the y-axis since there is space between ticks on the x-axis.
• Consider adding a zoom out (and in) for the y-axis. Some of the signals are clipped at the default zoom and users may want to see the peaks.

[Lucas-Mc]

Here's some initial thoughts in response @briangow :

• When saving to a .png, it may be nice to add a small title indicating the database and record number.

Looks like I can edit the file name, but maybe not at the final level, only initially (maybe something like demobsn.png? hopefully I can get more specific) ... I'll have to take a look further

• When zooming in on the time axis the xlabel ticks disappear eventually, perhaps consider creating new finer grain ticks when zooming in.

Hmmmm, might not be worth the effort since the x-axis ticks are set to be every 0.2 seconds and by then the downsampling would make it less than desirable... can you think of an application where I would need signals of that time scale?

• Autoscale and reset axis seem to do the same thing, which sounds like the expected behavior if a custom axes range isn't set (https://plotly.com/chart-studio-help/getting-to-know-the-plotly-modebar/). Do you plan to make use of this custom axes option? If not, is it possible to remove the reset axis button?

This has been something on my to-do list for a while now but I can't seem to figure it out in the Python version only the JS version (this motivated me to seriously start considering a WFDB-JS if I have spare time)... It seems like a package specific issue that a lot of people have been complaining about but I think it's in the "we need more funding to get it done" stage unfortunately... Maybe I should just remove one of them for now so it's not confusing

• Waveforms with annotations show the annotation at the top of the plot but since there isn't a line or dot that connects to the waveform itself, you can't tell exactly which sample the annotation applies to. Consider adding a dot or line on the waveform to make this more precise.

This functionality was in the big commented blob in the annotation generator in waveform_vis.py : def update_graph() but I removed it since it seemed to be much slower for a large number of annotations and I figured the user could just hover over the graph and that vertical dashed line would show up and it lines it up while giving them specific values at the same time... I'll keep it on my radar though!

• With 8 signals plotted the labels get very small and since they are also a gray color they become hard to read. Consider making the label pure black, or bold. You could also make the x-axis / time labels larger than the y-axis since there is space between ticks on the x-axis.

I see now... It looks like the color is set to rgb(43,63,95) by default so I'll look into changing it... I never noticed the x-axis labels getting smaller like that I think I accidentally made them a function of n_sig just like I did with the y-axis font size too so I'll switch that [FIXED]

• Consider adding a zoom out (and in) for the y-axis. Some of the signals are clipped at the default zoom and users may want to see the peaks.

This is a can of worms... How would I make it clean on a graph by graph basis if 8 signals are plotted at once? I think this would also make the manual zoom selection be a 2D rectangle with variable width and height instead of just width... Should I just relax the criteria for outliers or plot the whole signal with outliers and hope for the best?

[briangow]

Just a few follow-up thoughts on some of the points @Lucas-Mc. Again, I don't think any of these should be taken as necessary for the initial release of this waveform viewer.

• When zooming in on the time axis the xlabel ticks disappear eventually, perhaps consider creating new finer grain ticks when zooming in.

  • Hmmmm, might not be worth the effort since the x-axis ticks are set to be every 0.2 seconds and by then the downsampling would make it less than desirable... can you think of an application where I would need signals of that time
    • Are you setting the tick width based on the sampling rate? That seems like a good idea. Of course there are lots of signals where it'd be nice to have ticks closer than every 0.2 s. EEG signals can be 30+ Hz for example. Even with ECG's it could be convenient to have ticks a bit closer than 0.2 so you can quickly eyeball the time differences between the different peaks within a beat.

• Autoscale and reset axis seem to do the same thing, which sounds like the expected behavior if a custom axes range isn't set (https://plotly.com/chart-studio-help/getting-to-know-the-plotly-modebar/). Do you plan to make use of this custom axes option? If not, is it possible to remove the reset axis button?

  • This has been something on my to-do list for a while now but I can't seem to figure it out in the Python version only the JS version (this motivated me to seriously start considering a WFDB-JS if I have spare time)... It seems like a package specific issue that a lot of people have been complaining about but I think it's in the "we need more funding to get it done" stage unfortunately... Maybe I should just remove one of them for now so it's not confusing
    • Yeah, I think it would be better to remove the reset axes if it isn't going to be used to set custom axes.

• Waveforms with annotations show the annotation at the top of the plot but since there isn't a line or dot that connects to the waveform itself, you can't tell exactly which sample the annotation applies to. Consider adding a dot or line on the waveform to make this more precise.

  • This functionality was in the big commented blob in the annotation generator in waveform_vis.py : def update_graph() but I removed it since it seemed to be much slower for a large number of annotations and I figured the user could just hover over the graph and that vertical dashed line would show up and it lines it up while giving them specific values at the same time... I'll keep it on my radar though!
    • I wonder if it's possible to do something where if the user clicks a certain spot (near annotation marker) or enters a certain key sequence that the annotation line will appear. If it was on-demand, it wouldn't slow down plotting in general but could be accessed if needed.

• Consider adding a zoom out (and in) for the y-axis. Some of the signals are clipped at the default zoom and users may want to see the peaks.

  • This is a can of worms... How would I make it clean on a graph by graph basis if 8 signals are plotted at once? I think this would also make the manual zoom selection be a 2D rectangle with variable width and height instead of just width... Should I just relax the criteria for outliers or plot the whole signal with outliers and hope for the best?
    • I wasn't proposing any changes to your default zoom, only the ability for the user to zoom out on the y-axis, especially for cases where the waveform is clipped. I think it'd be fine to zoom out of all of the plotted waveforms by the same amount (percentage).

[tompollard]

Just as a reminder, latest discussion around this pull request was asking what kind of load it would place on our servers. Needs some investigation!