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

Start transition to quartodoc #198

Merged
merged 52 commits into from
Jul 25, 2023
Merged

Start transition to quartodoc #198

merged 52 commits into from
Jul 25, 2023

Conversation

juliasilge
Copy link
Member

No description provided.

@juliasilge
Copy link
Member Author

juliasilge commented May 23, 2023

@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
Copy link
Collaborator

machow commented Jun 1, 2023

@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:

  • What's a function, and what's a method? Reading pin_list() on the page makes it feel like it's a top level function. At the same time, writing it as BaseBoard.pin_list() feels a bit cumbersome. I wonder if us naming the section "Pin Methods" might help? But overall think it's a very useful area to discuss, since it lives in that funky "translating pkgdown to python" space.
  • It feels like something is missing to see constructors like board_s3() but not a board object like BaseBoard listed. In the original docs I just stuffed it in with the methods. We can use quartodoc to create a similar BaseBoard page if we want. I'm still trying to get a feel for the right move in these kinds of situations. For vetiver we include the class VetiverAPI on the reference page, along with its methods).

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
Copy link
Member Author

@machow So great to see those changes integrated!

  • I added descriptions to say that we have "Constructors for board objects" and then "Pin methods for your board".
  • 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?
  • One more question on formatting -- how can we get the links on this page to be code where appropriate, i.e. board_folder and not "board_folder" in regular text?

Screenshot 2023-06-01 at 10 58 22 AM

@machow
Copy link
Collaborator

machow commented Jun 1, 2023

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
Copy link
Member Author

  • I updated the sections on the main reference index the way we chatted about @machow, to clarify that everything in the "Pins" section is a method.
  • I did remove the sidebar and it's a big improvement (clearer/cleaner and less confusing). We do still have page navigation at the bottom to click through all the boards/pins pages, which I think is OK to keep (not so distracting).

@machow
Copy link
Collaborator

machow commented Jun 6, 2023

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
Copy link
Member Author

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
Copy link
Collaborator

machow commented Jun 6, 2023

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

@machow
Copy link
Collaborator

machow commented Jun 21, 2023

TODO:

  • change "Function Reference" to Reference
  • remove dev branch of quartodoc (should be on main)

@juliasilge
Copy link
Member Author

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

@machow machow marked this pull request as ready for review July 25, 2023 21:03
@machow
Copy link
Collaborator

machow commented Jul 25, 2023

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

@machow machow merged commit ca9dde2 into main Jul 25, 2023
@juliasilge juliasilge deleted the quartodoc-attempt-1 branch October 20, 2023 15:45
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