Skip to content
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

Improve Documentation VII #2229

Merged
merged 8 commits into from
Nov 22, 2023
Merged

Improve Documentation VII #2229

merged 8 commits into from
Nov 22, 2023

Conversation

zm711
Copy link
Collaborator

@zm711 zm711 commented Nov 18, 2023

So in this documentation improvement I did:

  1. My usual search for typo fixes/minor clarifications
  2. I rearranged the sorters docs to move the internal sorters above the legacy sorters because as a reader once I reach legacy I would likely stop reading the docs rather than read on to see SC2 and Tridesclous2, so it is better to be proud of the internal products and move them above legacy. I also added install instruction for Tridesclous2 so for people that want to test it out they can
  3. I deleted the legacy section of widgets documentation and instead moved/made sure that every widget currently exposed in the widgets_list.py is connected to in the docs. My question for that is: I added matplotlib backend, but do all widgets now all have at least the ipy backend as well
  4. I actually went through the examples for the modules gallery to fix typos and try to add clarifications and keyword arguments

@zm711 zm711 added the documentation Improvements or additions to documentation label Nov 18, 2023
@alejoe91
Copy link
Member

Thank you so much Zach!!! This is great :)

No not all widgets have a ipy backend yet. For example:

  • plot_all_amplitudes_distributions
  • plot_crosscorrelograms
  • plot_agreement_scores

The available backends are correctly listed in the API references: https://spikeinterface.readthedocs.io/en/latest/api.html#spikeinterface.widgets.plot_all_amplitudes_distributions

@zm711
Copy link
Collaborator Author

zm711 commented Nov 20, 2023

Of course :)

The available backends are correctly listed in the API references:
https://spikeinterface.readthedocs.io/en/latest/api.html#spikeinterface.widgets.plot_all_amplitudes_distributions

Cool, I'll update the backend part of the docs soon.

I wanted to update this because I constantly go to that section of the docs to remember the exact name of the plotting function I want (for example the name change from multicomp to multicomparison etc).

@alejoe91
Copy link
Member

alejoe91 commented Nov 20, 2023

Of course :)

The available backends are correctly listed in the API references:
https://spikeinterface.readthedocs.io/en/latest/api.html#spikeinterface.widgets.plot_all_amplitudes_distributions

Cool, I'll update the backend part of the docs soon.

I wanted to update this because I constantly go to that section of the docs to remember the exact name of the plotting function I want (for example the name change from multicomp to multicomparison etc).

@zm711 since you're at it, could you remove the Legacy part of the widgets API (https://spikeinterface--2229.org.readthedocs.build/en/2229/api.html#legacy-widgets) and check if all plot_functions are listed there?

List of plot functions

API entries

Thanks!!!

@zm711
Copy link
Collaborator Author

zm711 commented Nov 20, 2023

Yep. I can do that as well. It's a holiday coming up here so it might not happen this week, but I'll put it on my todo list.

@alejoe91
Copy link
Member

No rush! I can also try to add them if you're ok with me pushing here :)

@zm711
Copy link
Collaborator Author

zm711 commented Nov 20, 2023

No rush! I can also try to add them if you're ok with me pushing here :)

Of course. Always.

@zm711
Copy link
Collaborator Author

zm711 commented Nov 20, 2023

Just one note if you do push changes, just watch out editing core because I didn't build this on top of my new import docs with core edited so I'm hoping to avoid merge conflicts... (I know you were just planning on looking at api.rst, but just as a warning).

@alejoe91
Copy link
Member

Done and also fixed some other sphinx warnings and missing refs :)

doc/modules/sorters.rst Outdated Show resolved Hide resolved
@@ -70,15 +71,15 @@ To install it, run:

.. code-block:: bash

pip install sortingview figurl-jupyter
pip install sortingview
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice catch!

@@ -16,15 +16,15 @@ spikeinterface.widgets

The easiest way to visualize :code:`spikeinterface` objects is to use the :code:`widgets` module for plotting.
You can find an extensive description in the module documentation :ref:`modulewidgets`
and many examples in this tutorial :ref:`sphx_glr_modules_gallery_widgets`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For context, this was generating a missing reference and I couldn't find a way to ref a dynamically generated sphinx-gallery module, but just individual files.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A lot of the sphinx gallery stuff is annoying like that, but based on my reading they don't really have plans to try to patch things like that. Such is life I suppose.

Copy link
Collaborator Author

@zm711 zm711 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple more typos while we are at it.

doc/modules/widgets.rst Outdated Show resolved Hide resolved
doc/modules/core.rst Outdated Show resolved Hide resolved
doc/modules/core.rst Outdated Show resolved Hide resolved
doc/modules/core.rst Outdated Show resolved Hide resolved
@alejoe91 alejoe91 merged commit bf1bb9a into SpikeInterface:main Nov 22, 2023
11 checks passed
@zm711 zm711 deleted the typo-docs branch November 22, 2023 11:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants