-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Fix versionadded directive formatting
#13471
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
Conversation
|
FYI if you want to go the extra mile... we could hook into the Sphinx event system and make sure any https://www.sphinx-doc.org/en/master/extdev/event_callbacks.html#event-source-read and look at We could add the same for And we probably never want the regular expression Feel free to give it a shot here, otherwise just create an issue from this comment with a suitable title and someone can work on it at some point when they're motivated |
|
Happy to give this a go. Would the appropriate way be to add a new script to |
|
I would connect it to |
|
Ah sorry, didn't realise |
This reverts commit f48c110.
|
Having issues building the docs locally, so reverting the earlier fixes and want to see if they get picked up by the new script here. |
|
Okay, so the script can detect cases of bad formatting in e.g., examples and tutorials (and is picking up an instance I wasn't aware of before), but the content from |
|
@larsoner Is there a way to access the docstrings at the read phase, or do we have to rely on e.g., checking the html files once they've been written? |
|
Sorry for the delay, had to fight some new sphinx-build - Windows compatibility issues I did not have before.
That was exactly it, thanks @larsoner! Have updated the script so it'll also check docstrings. Couple things to note which I will look into later:
|
|
It's also picking up this case in the examples where the references aren't properly separated from the code. mne-python/examples/preprocessing/css.py Lines 94 to 105 in 84bc971
This gives the following warning: |
I think it's okay if the warning isn't perfect / correct, as long as there is something to actually fix (and there is here!). What would be good to avoid is false alarms |
|
... and if there are some false alarms, we can build some mechanism / allowlist of exceptions. But maybe it wont be needed yet, we'll see! |
Everything that gets picked up now is a genuine case of the directive not rendering properly.
The exceptions I found so far where a blank line is not necessary are:
mne-python/doc/sphinxext/directive_formatting.py Lines 46 to 66 in 84bc971
|
|
The warnings/errors look great! I think you can fix them and we can merge |
|
Just pushed a commit that:
Warnings are still the same (https://app.circleci.com/pipelines/github/mne-tools/mne-python/29136/workflows/b74c3e86-2455-40c9-ba7b-384d5796bb65/jobs/76730/parallel-runs/0/steps/0-143), will fix these cases now. |
|
Warnings are fixed now! |
* upstream/main: (230 commits) FIX: Fix ICA.apply when fitted including marked bad channels (mne-tools#13478) FIX: Correctly set the calibration factor in Nihon Kohden reader (mne-tools#13468) [pre-commit.ci] pre-commit autoupdate (mne-tools#13479) MAINT: Update code credit (mne-tools#13477) Fix `versionadded` directive formatting (mne-tools#13471) typo in mailmap (mne-tools#13475) FIX: Fix _plot_topomap channel names plotting when using a mask (mne-tools#13470) FIX: Handle an Eyelink File with blank lines injected throughout file (mne-tools#13469) ENH: adds annotation filtering to raw and ica source figures (mne-tools#13460) MAINT: Restore VTK nightly wheel on Linux and bump changelog checker (mne-tools#13436) [pre-commit.ci] pre-commit autoupdate (mne-tools#13465) FIX: Fix add_reference_channels for passing two channels names (mne-tools#13466) ENH: Add on_missing for combine_channels (mne-tools#13463) Bump the actions group with 2 updates (mne-tools#13464) Move development dependencies into a dependency group (no more extra) (mne-tools#13452) ENH: add on_missing for rename_channels (mne-tools#13456) add advisory board to website (mne-tools#13462) ENH: Support Nihon Kohden EEG-1200A V01.00 (mne-tools#13448) MAINT: Update dependency specifiers (mne-tools#13459) ENH: Add encoding parameter to Nihon Kohden reader (mne-tools#13458) ...
* upstream/main: (85 commits) FIX: Fix ICA.apply when fitted including marked bad channels (mne-tools#13478) FIX: Correctly set the calibration factor in Nihon Kohden reader (mne-tools#13468) [pre-commit.ci] pre-commit autoupdate (mne-tools#13479) MAINT: Update code credit (mne-tools#13477) Fix `versionadded` directive formatting (mne-tools#13471) typo in mailmap (mne-tools#13475) FIX: Fix _plot_topomap channel names plotting when using a mask (mne-tools#13470) FIX: Handle an Eyelink File with blank lines injected throughout file (mne-tools#13469) ENH: adds annotation filtering to raw and ica source figures (mne-tools#13460) MAINT: Restore VTK nightly wheel on Linux and bump changelog checker (mne-tools#13436) [pre-commit.ci] pre-commit autoupdate (mne-tools#13465) FIX: Fix add_reference_channels for passing two channels names (mne-tools#13466) ENH: Add on_missing for combine_channels (mne-tools#13463) Bump the actions group with 2 updates (mne-tools#13464) Move development dependencies into a dependency group (no more extra) (mne-tools#13452) ENH: add on_missing for rename_channels (mne-tools#13456) add advisory board to website (mne-tools#13462) ENH: Support Nihon Kohden EEG-1200A V01.00 (mne-tools#13448) MAINT: Update dependency specifiers (mne-tools#13459) ENH: Add encoding parameter to Nihon Kohden reader (mne-tools#13458) ...

What does this implement/fix?
Noticed some of the
versionaddedSphinx directives were not rendering properly in the docs. Checked also for other directives while I was at it, but I only saw this with cases ofversionadded.