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

docs: implement autoapi #269

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open

docs: implement autoapi #269

wants to merge 14 commits into from

Conversation

12rambau
Copy link
Contributor

@12rambau 12rambau commented Aug 22, 2023

related to #268

implement the documentation of the API using sphinx-autoapi extention. all the members are exposed but none of them is documented (follow-up PR).

technical choices

  • I selected sphinx-autoapi instead of autodocs as its faster and don't need extra build steps
  • I solved some small ruff error by adding the init parameter (import from init are not treated the same way) and applied the suggestions. Without it, autoapi was looping forever.
  • I decided to hide private members (as most of ipyvuetify files are generated)
  • members are grouped by type
  • if any typhint is provided it will be shown in the descriptions

side note

The build is currently ugly as all the classes goes mostly to the same file but with the left sidebar of pydata-sphinx-theme it will be way easier to navigate. There is also an open-issue on autoapi side to split all class in independat file: readthedocs/sphinx-autoapi#226 a PR is under review.

Issue

this is where a technical descision needs to be taken. To hide private method/class/package/module, they need to respect the PEP convention: have a trailing underscore. It's not the case in any of them.
I identified 3 packages that are not exposed to the end user:

  • generated
  • VuetifyTemplate
  • Html

To my understanding it should have been harmless to change their names (which I did) but now the tests are failing because reacton (and by extend solara) is looking for the generated folder. What should I do ?

On the one side I agree it may impact lots of projects (but I doubt anyone else than @mariobuikhuizen and @maartenbreddels know about the internal structure) but I would also like to avoid its spread by signaling explicitely to end users that this packages are internals (sphinx is currently facing this issue and everyt time they remove a "private" method they realise an existing theme is using it).

what are your advices ?

@12rambau 12rambau marked this pull request as ready for review August 23, 2023 11:16
@mariobuikhuizen
Copy link
Collaborator

mariobuikhuizen commented Aug 24, 2023

Maybe this can be used instead of renaming files.directories?: https://sphinx-autoapi.readthedocs.io/en/latest/how_to.html#how-to-customise-what-gets-documented

@12rambau
Copy link
Contributor Author

I'm going to try your suggestion but as a side note I also tried another mechanism from my side:

if you add:

# conf.py
autoapi_ignore = ["*Html*", "*VuetifyTemplate*", "*generated*"]

They are effectively skiped but then you get the following errors:

WARNING: Cannot resolve import of unknown module ipyvuetify.generated in ipyvuetify
WARNING: Cannot resolve import of unknown module ipyvuetify.Html in ipyvuetify
WARNING: Cannot resolve import of unknown module ipyvuetify.VuetifyTemplate in ipyvuetify

And all the widgets are ignored.

@12rambau
Copy link
Contributor Author

Ok so the skip event was the way to go. It's only disadvantage is that the files are read and build anyway, they are just not integrated to the toctree so the build still take ages due to the number of stub pages in "generated".

@12rambau 12rambau marked this pull request as draft August 25, 2023 07:46
@12rambau 12rambau marked this pull request as ready for review August 25, 2023 08:19
@12rambau
Copy link
Contributor Author

12rambau commented Sep 7, 2023

@mariobuikhuizen ready to merge !

@12rambau
Copy link
Contributor Author

@mariobuikhuizen, @maartenbreddels, anything I should change for this one (apart from solving the conflict)?
I need this to be merged to start working on the docstring generation from Vuetify sources.

@12rambau
Copy link
Contributor Author

I saw some activity from the project on my notification dashboard so my question will be super simple, am I being ghosted?
You specifically requested from me to reduce the footprint of my PR, which I did, slicing the documentation process in smaller pieces that are unfortunately interconnected. I'm now blocked for 6 months. If you don't want external help, let me know I'll do my own stuff in my branch and stop pinging you.

@mariobuikhuizen
Copy link
Collaborator

Hi @12rambau,

We are not ghosting you, we have just limited time and this couldn't be a top priority and then slipped off the radar.

We do appreciate the help!

Note that you can work on the docstring generation without this being merged in master, you can just base that branch/PR on this one. Or the other way around, basing this PR on the docstring generation branch/PR since this PR might need changes to incorporate the docstrings.

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.

2 participants