Stairs plot

[remrama]

Makes a minor adjustment to the underlying yasa.plot_hypnogram code. The hypnogram line is now drawn using a combination of plt.stairs and plt.hlines instead of plt.step. See Issue #106 for motivation behind this change.

Also adds a fill_color argument yasa.plot_hypnogram for an optional solid-color fill above the hypnogram line. Default value (None) will not draw filling.

[remrama]

Note that I only used ax.hlines for the REM and Art/Uns highlighting because of the poor handling of masked arrays in ax.stairs.

[raphaelvallat]

@remrama you're amazing!!

• Can you please also copy-paste these changes to the yasa.plot_spectrogram function (I'd say without the fill_color argument)?

  yasa/yasa/plotting.py

  Lines 279 to 281 in 399d984

  	# Hypnogram (top axis)
  	ax0.step(t_hyp, -1 * hypno, color="k")
  	ax0.step(t_hyp, -1 * hypno_rem, color="r")

• Can you please copy-paste here the generated docstring plots?

[raphaelvallat]

And edit the changelog please! Version 0.6.3.dev

[remrama]

No problem!

wrt the yasa.plot_spectrogram code, are you cool with instead replacing the fig argument with ax in yasa.plot_hypnogram and just using yasa.plot_hypnogram(..., ax=ax0) under the hood of yasa.plot_spectrogram? I've glanced at it and I don't foresee problems. Of course this avoids redundancies and is in general more flexible.

[raphaelvallat]

Agreed, good idea!

[codecov-commenter]

Codecov Report

Base: 91.41% // Head: 91.43% // Increases project coverage by +0.02% 🎉

Coverage data is based on head (bf89ed8) compared to base (b98a4c0).
Patch coverage: 100.00% of modified lines in pull request are covered.

❗ Current head bf89ed8 differs from pull request most recent head 7a10187. Consider uploading reports for the commit 7a10187 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #108      +/-   ##
==========================================
+ Coverage   91.41%   91.43%   +0.02%     
==========================================
  Files          22       22              
  Lines        2690     2674      -16     
==========================================
- Hits         2459     2445      -14     
+ Misses        231      229       -2     

Impacted Files	Coverage Δ
yasa/plotting.py	98.69% <100.00%> (+1.59%)	⬆️
yasa/tests/test_plotting.py	100.00% <100.00%> (ø)
yasa/tests/test_spectral.py	100.00% <100.00%> (ø)
yasa/detection.py	97.73% <0.00%> (-0.12%)	⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[remrama]

Should be good now @raphaelvallat! Attaching a figure that should show up during docs, or did you want the actual docs-generated one?

A few notes:

• Good thing you asked for an update on the spectrogram plot. I ran into an issue plotting with ax.stairs because the sampling frequency is so much higher in that example! Previously, I was drawing a separate line for every epoch -- even if they were the same for a bunch of continuous epochs. That is very slow with a high sampling frequency. So I added a small section to yasa.plot_hypnogram that now finds all the continuous epochs and just draws one line per every break-point in the hypnogram. It's a good catch.
• I hope it's alright I consolidated the code of yasa.plot_spectrogram a bit. Now that you don't have to do all the hypnogram work, it's a pretty straight-forward function. Actually, I think in the future you might drop the hypno option and just have an example in the docs where you open a figure with 2 axes and then yasa.plot_spectrogram(ax=ax1) and yasa.plot_hypnogram(ax=ax0). Not a big deal so just leaving it for now.
• I saw there were no unittests for yasa.plot_spectrogram. Maybe there's a reason for that so I left it alone, but lmk if you want me to add some.

[raphaelvallat]

Awesome, looks great! I'll do an in-depth review of the code soon. FYI there are unit tests for the plot_spectrogram function:

yasa/yasa/tests/test_spectral.py

Lines 152 to 169 in 399d984

	def test_plot_spectrogram(self):
	"""Test function plot_spectrogram"""
	plot_spectrogram(data_full[0, :], sf_full, fmin=0.5, fmax=30)
	plot_spectrogram(data_full[0, :], sf_full, hypno_full, trimperc=5)
	plot_spectrogram(data_full[0, :], sf_full, fmin=0.5, fmax=30, vmin=-50, vmax=100)
	hypno_full_art = np.copy(hypno_full)
	hypno_full_art[hypno_full_art == 3.0] = -1
	# Replace N3 by Artefact
	plot_spectrogram(data_full[0, :], sf_full, hypno_full_art, trimperc=5)
	# Now replace REM by Unscored
	hypno_full_art[hypno_full_art == 4.0] = -2
	plot_spectrogram(data_full[0, :], sf_full, hypno_full_art)
	plt.close("all")
	# Errors
	with pytest.raises(AssertionError):
	plot_spectrogram(data_full[0, :], sf_full, fmin=0.5, fmax=30, vmin=-50)
	with pytest.raises(AssertionError):
	plot_spectrogram(data_full[0, :], sf_full, fmin=0.5, fmax=30, vmax=100)

[raphaelvallat]

Hi @remrama,

Quick question: When using fill_color with an hypnogram that contains ART and UNS, wouldn't it be better not to fill the ART and UNS? Currently the output looks like:

import numpy
import yasa
import matplotlib.pyplot as plt
hypno = 100 * [-2] + list(np.repeat([-0, 0, -1, -1, 0, 0, 1, 2, 2, 3, 3, 3, 4, 4, 4, 4, 0, 0, 0], 120)) + 100 * [-2]
_, ax = plt.subplots(1, 1, figsize=(9, 4))
yasa.plot_hypnogram(hypno, sf_hypno=1/10, lw=1.5, fill_color="gainsboro");

That said I don't have a strong preference here.

[raphaelvallat]

Also, do you have a preference for using

plt.figure(figsize=(7, 3), constrained_layout=True)
ax = yasa.plot_hypnogram(hypno, fill_color="gainsboro")

or

fig, ax = plt.subplots(1, 1, figsize=(7, 3))
yasa.plot_hypnogram(hypno, fill_color="gainsboro", ax=ax)

[remrama]

Good call on the Art/Uns. There was a simple fix, clipping the hypno to only wake+ epochs when passing to the ax.stairs used for filling. Resulting pic attached.

wrt returning ax or not, no I don't have a preference. I've switched it up to return and pass ax through. Though I think I do have a small preference for leaving constrained_layout or adding plt.tight_layout() as a final line. Without one of these, the x-axis label is off the figure, and I think it's better for users to see an example that is pretty without needing adjustments. I left it there but would still change it if you'd like.

[raphaelvallat]

Perfect! Almost there, I just added one last comment re: order of the code. Thanks again!

[remrama]

K pushed up that change.

btw, I didn't do any rebasing/merging with master (1 commit behind). I figure you can handle that in this instance, but for future reference, do you have a preference on whether I should rebase vs merge master into whatever my feature-branch is before submitting a PR? Not sure how you like the commit history to flow. Or is this highly situation-dependent and I should figure it out myself...

[raphaelvallat]

Rebase whenever possible. Thanks so much again, merging now!