rstudio / pins-python

https://rstudio.github.io/pins-python/
MIT License
52 stars 12 forks source link

Start transition to quartodoc #198

Closed juliasilge closed 1 year ago

juliasilge commented 1 year ago

@machow I am seeing this error when I try to run quartodoc build, and I'm not sure what I changed since it was working earlier:

TypeError: dataclass_transform() got an unexpected keyword argument 'field_specifiers'

Does that look familiar?

Edited to say this was the version of Pydantic. I think I ran into this after installing from the dev requirements.txt in this package.

machow commented 1 year ago

@juliasilge I think ipython 8.13 dropped support for python 3.8, so we would need to install 8.12

https://ipython.readthedocs.io/en/stable/whatsnew/version8.html#ipython-8-12-1

juliasilge commented 1 year ago

@machow If I install ipython 8.12.0 locally, then doing pip-compile --extra=doc --extra=test --output-file=- setup.cfg still puts 8.13.2 into requirements/dev.txt. I think it must be something else in setup.cfg that is causing that version of ipython to be chosen.

Edited to say that this works 🎉 but quartodoc requires 3.9 or higher. I bumped the Python version to 3.9 for the remaining failing checks. ⤵️

juliasilge commented 1 year ago

I did not add any links to things like fsspec.filesystem because there is not much available.

juliasilge commented 1 year ago

This is looking pretty good, I think!

@machow here are some blockers to moving forward at this point:

🎯 When using interlinking, the generated links look like regular text instead of code:

Screenshot 2023-05-25 at 1 07 33 PM

We need them to look like [`board_folder()`](/reference/board_folder.html)

🎯 I'm having trouble using interlinks for parameters:

Screenshot 2023-05-25 at 1 40 41 PM

juliasilge commented 1 year ago

Will close #186 if we can get interlinks to look OK

machow commented 1 year ago

Shoot--I think that not generating a link inside the parameter description is a bug. We currently sanitize the output, so that symbols like | won't break the markdown table, but this ends up also screwing up the interlink syntax...

Opened an issue here https://github.com/machow/quartodoc/issues/172

machow commented 1 year ago

@juliasilge it looks like the latest quartodoc changes are working: the interlinks are now wrapped inside code elements, and can render in a parameter table description.

One tricky thing with pins (related to figuring out analogue to pkgdown) seems like how we convey two pieces:

Here's the relevant BaseBoard page on the current pins docs

image

edit: here's how vetiver lays out the class and methods on the reference page

image

juliasilge commented 1 year ago

@machow So great to see those changes integrated!

Screenshot 2023-06-01 at 10 58 22 AM

machow commented 1 year ago

I feel fuzzy on what adding documentation for BaseBoard gets us that isn't already there in the sidebar. Can you tell me a little bit more about what we might document on a page like that?

It feels like some kind of connective tissue is missing. There are a bunch of function/method names on the page, but as a person who will be interacting with an object, it's surprising the object type itself is not documented / named.

The idea that some functions return an object, and then some other things that look like functions are actually methods on an object--without the object there--feels like it's missing what l see in the repl / various informative displays.

image

RE "what we might document on a page like that?"

I think this situation is similar to documenting an S4 / R6 class, so just having the type name on the sidebar will reinforce where pin_read() lives. (On the page itself we could just emphasize that this is the main object people interact with in the package; repeat some of the other constructor stuff).

Alternatively

If we are very loud in saying some things construct a BaseBoard instance, some things are methods, we might be able to get the point across.

juliasilge commented 1 year ago
machow commented 1 year ago

Thanks for these changes!

Do you mind saying more about when you think removing the sidebar makes things more clear? I'm onboard with any changes, and want to make sure I capture the strategy (eg why removing the sidebar might provide clarity here compared to the vetiver api docs).

It seems like the big factor might just be the size of the pins api docs (since it's just documentation around roughly a single class, with only a few methods)?

juliasilge commented 1 year ago

I would likely vote for removing the sidebar on the vetiver reference page too, to make more pleasant and focused pages, but I do think for pins it's harder to argue it adds much helpful info (a small package, like you said) relative to the added visual busy-ness.

The biggest challenge is that a sidebar full of either styled code (PURPLE! or GREEN! or what the theme has) or plain text ("which ones of these are words?") doesn't look very good/clear and so is distracting. Maybe in the long run, some more pleasant and subtle styling of the sidebar would be possible.

machow commented 1 year ago

Thanks, this is super helpful feedback--especially about being in favor of removing the vetiver sidebar.

machow commented 1 year ago

TODO:

juliasilge commented 1 year ago

Also TODO here is to switch to the regular /docs folder, and replace the existing docs.

machow commented 1 year ago

Okay, finally wrapped the last TODO pieces w/ @isabelizimm , merging!