Skip to content

Tweak y-axis label#13298

Merged
drammock merged 15 commits intomne-tools:mainfrom
cbrnr:psd-ylabel
Jul 5, 2025
Merged

Tweak y-axis label#13298
drammock merged 15 commits intomne-tools:mainfrom
cbrnr:psd-ylabel

Conversation

@cbrnr
Copy link
Copy Markdown
Contributor

@cbrnr cbrnr commented Jun 25, 2025

Use more common y-axis label for PSD plot. Fixes #13296.

@cbrnr
Copy link
Copy Markdown
Contributor Author

cbrnr commented Jun 25, 2025

@drammock I tried to make the docstring slightly more readable, but the dB_plot_psd entry is just for the legacy function(s). The new functions are based on the _dB entry, which currently is

_dB = """
dB : bool
    Whether to plot on a decibel scale. If ``True``, plots
    10 × log₁₀({quantity}){caveat}.{extra}
"""

I would like to change the following three things:

  1. The factor 10 only applies for PSDs, but for ASDs the factor is 20 (hopefully that's also what the code does, I didn't check).
  2. The formula shows the conversion to dB, but the plots (both PSD and ASD) show dB/Hz or dB/√Hz. I think this should be included in the docs, but I'm not sure how.
  3. In that docstring we could also include the reference power or amplitude values, but I'm also not sure how.

@cbrnr cbrnr marked this pull request as ready for review July 3, 2025 07:36
@cbrnr
Copy link
Copy Markdown
Contributor Author

cbrnr commented Jul 3, 2025

I think this is good to go even without the three minor tweaks to the docs.

@drammock
Copy link
Copy Markdown
Member

drammock commented Jul 3, 2025

The factor 10 only applies for PSDs, but for ASDs the factor is 20 (hopefully that's also what the code does, I didn't check).

The code handles the 10 vs 20 coefficient here, a few lines up from the lines you edited in this PR.

The formula shows the conversion to dB, but the plots (both PSD and ASD) show dB/Hz or dB/√Hz. I think this should be included in the docs, but I'm not sure how.

We never pass anything to scipy.signal.spectrum's scaling param, which means it's always "density" and therefore always /Hz (or /sqrt(Hz)). I've tried to clarify this in the docstring of dB.

In that docstring we could also include the reference power or amplitude values, but I'm also not sure how.

I think we can't easily do that actually, as the reference value units will change for mags, grads, or EEG. I think we really do need to keep the reference value in the y-axis label.

@cbrnr
Copy link
Copy Markdown
Contributor Author

cbrnr commented Jul 3, 2025

Thanks @drammock! I'm not happy with keeping the reference in the label, but it's at least better than before. Can you remove the colon so that at least there's one less character cluttering the label? Thanks also for helping with the other points.

@cbrnr
Copy link
Copy Markdown
Contributor Author

cbrnr commented Jul 3, 2025

Feel free to add yourself to the changelog BTW!

@drammock
Copy link
Copy Markdown
Member

drammock commented Jul 4, 2025

@cbrnr FYI force-pushing here makes it a bit harder to figure out why the tests were failing (as we lose easy access to CI logs from past commits through the GitHub UI). not sure exactly why you force-pushed, but maybe next time git pull --rebase will prevent the need for it.

@cbrnr
Copy link
Copy Markdown
Contributor Author

cbrnr commented Jul 4, 2025

Yeah, I force push out of habit after rebasing, but I'll keep that in mind next time!

@drammock drammock merged commit 5009ee8 into mne-tools:main Jul 5, 2025
32 checks passed
@cbrnr cbrnr deleted the psd-ylabel branch July 7, 2025 05:06
WouterKroot pushed a commit to WouterKroot/mne-python that referenced this pull request Aug 13, 2025
Co-authored-by: Daniel McCloy <dan@mccloy.info>
zEdS15B3GCwq pushed a commit to zEdS15B3GCwq/mne-python that referenced this pull request Aug 25, 2025
Co-authored-by: Daniel McCloy <dan@mccloy.info>
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.

Further clarify dB units in PSD plots

2 participants