search

mne-tools / mne-connectivity

Connectivity algorithms that leverage the MNE-Python API.
https://mne.tools/mne-connectivity/dev/index.html
BSD 3-Clause "New" or "Revised" License
65 stars 34 forks source link

`spectral_connectivity_time` method list not rendering properly #170

Closed tsbinns closed 5 months ago

tsbinns commented 5 months ago

mne_connectivity.spectral_connectivity_time() has a nice overview of the supported connectivity methods under the methods parameter: https://github.com/mne-tools/mne-connectivity/blob/043d88873b968f948e9571237c5f4ef8e0a4cef4/mne_connectivity/spectral/time.py#L71-L85


Looking at the code I'm guessing this is supposed to be a list of bullet points, however it does not render this way: image

For this to render properly, there should be a new line before and after the list.

How would people feel about a small PR to add these new lines?


Related point: I like this way of giving a quick overview of the supported methods. Do you think it would be worth adding the same in mne_connectivity.spectral_connectivity_epochs()?

larsoner commented 5 months ago

Yeah newlines would be good. +1 for adding to the epochs docstring as well, preferably by a deduplication / use of docs.py

tsbinns commented 5 months ago

@larsoner Given that the methods are slightly different between epochs and time, would it still be possible to deduplicate this using docs.py?

larsoner commented 5 months ago

Can you deduplicate some but not all of the methods? That might still be worth it. But if it's too much of a pain, duplicating the docstring is okay.

tsbinns commented 5 months ago

Didn't realise fill_doc was so flexible. Will open a PR with the changes.

larsoner commented 5 months ago

Yeah it's really just Python string substitution %s with a little bit of indentation magic. Hopefully the magic doesn't get in your way!

tsbinns commented 5 months ago

Looking at the render on my end it doesn't seem to break the other formatting. Might have to incorporate this in some other projects!

Opened a PR in #171.