-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Updating documentation to clarify full vs half-bandwidth and defaults in time_frequency.multitaper.py #11479
Conversation
Corrected the description of 'bandwitdh' parameter to properly reflect that it is the full bandwidth. Also correct the explanation of the default value of the bandwidth.
The second bandwidth mentioned in the ValueError when computing half_nbw is a half-bandwidth, updated to clarify
Hello! 👋 Thanks for opening your first pull request here! ❤️ We will try to get back to you soon. 🚴🏽♂️ |
Co-authored-by: Mathieu Scheltienne <[email protected]>
@britta-wstnr can you look and merge if happy? |
@larsoner the failing test is unrelated as far as i can see, right? |
Looks good to me too, thanks for the changes @tomdstone! |
Hi @tomdstone You can find a more thorough description about these steps on the MNE Contributor guide https://mne.tools/stable/install/contributing.html under the section "First-time contributors". |
@britta-wstnr the failing test should have been fixed by #11478 so I've gone ahead and merged main here, that should fix it. @tomdstone be sure to |
Changelog and author list updated! |
🎉 Congrats on merging your first pull request! 🥳 Looking forward to seeing more from you in the future! 💪 |
* upstream/main: (46 commits) Fix docstrings by replacing str with path-like and fix double backticks for formatting (mne-tools#11499) Use pathlib.Path instead of os.path to handle files and folders [circle deploy] (mne-tools#11473) MAINT: Fix Circle [circle deploy] (mne-tools#11497) MAINT: Use mamba in CIs (mne-tools#11471) Updating documentation to clarify full vs half-bandwidth and defaults in time_frequency.multitaper.py (mne-tools#11479) Fix typo in tutorial (mne-tools#11498) Typo fix and added colons before code (mne-tools#11496) [MRG] ENH/DOC: demo custom spectrum creation (mne-tools#11493) Accept only left-clicks for adding annotations (mne-tools#11491) [BUG, MRG] Fix pial surface loading, logging in locate_ieeg (mne-tools#11489) [ENH] Added unit_role to add non-breaking space between magnitude and units (mne-tools#11469) MAINT: Fix CircleCI build (mne-tools#11488) [DOC] Updated decoding.SSD documentation and internal variable naming (mne-tools#11475) Typo fix (mne-tools#11485) [MRG] Forward argument axes from plot_sensors to DigMontage.plot (mne-tools#11470) [MRG] Improve error message raised on channels missing from DigMontage (mne-tools#11472) MAINT: Deal with pkg_resources usage bugs (mne-tools#11478) Add object array support and docstring (mne-tools#11465) [ENH] Adjusted SSD algorithm to support non-full rank data (mne-tools#11458) [BUG] fix nibabel reference (mne-tools#11467) ...
* upstream/main: (264 commits) BUG: Fix deprecated API usage in example (mne-tools#11512) Deprecate 'kind' and 'path' in favor of 'fname' in the layout reader (mne-tools#11500) EGI/MFF events outside EEG recording should not break the code (mne-tools#11505) fixed annotations error on export (mne-tools#11435) DOC: Update installer links [skip azp] [skip actions] [skip cirrus] (mne-tools#11506) BUG: updates for MPL 3.7 compatibility (mne-tools#11409) Fix docstrings by replacing str with path-like and fix double backticks for formatting (mne-tools#11499) Use pathlib.Path instead of os.path to handle files and folders [circle deploy] (mne-tools#11473) MAINT: Fix Circle [circle deploy] (mne-tools#11497) MAINT: Use mamba in CIs (mne-tools#11471) Updating documentation to clarify full vs half-bandwidth and defaults in time_frequency.multitaper.py (mne-tools#11479) Fix typo in tutorial (mne-tools#11498) Typo fix and added colons before code (mne-tools#11496) [MRG] ENH/DOC: demo custom spectrum creation (mne-tools#11493) Accept only left-clicks for adding annotations (mne-tools#11491) [BUG, MRG] Fix pial surface loading, logging in locate_ieeg (mne-tools#11489) [ENH] Added unit_role to add non-breaking space between magnitude and units (mne-tools#11469) MAINT: Fix CircleCI build (mne-tools#11488) [DOC] Updated decoding.SSD documentation and internal variable naming (mne-tools#11475) Typo fix (mne-tools#11485) ...
Reference issue
Fixes #11324
What does this implement/fix?
Several folks noticed that the default bandwidth in
time_frequency.psd_multitaper()
was not documented correctly, and that the bandwidth was incorrectly documented as a half-bandwidth. The documentation has been updated to specify that it is the full frequency bandwidth, and the documentation of how the default bandwidth is 'computed'. The old version saying that the default bandwidth was 4 Hz was incorrect, as the 4 was the normalized time-half-bandwidth product.Also updated the
ValueError
in_compute_mt_params
when computinghalf_nbw
to specify which is full bandwidth and which is half-bandwidth.