-
Notifications
You must be signed in to change notification settings - Fork 22
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
DOC: basic API docs #18
Conversation
This wasn't as straightforward as I hoped but it builds now https://pyogrio--18.org.readthedocs.build/en/18/index.html |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @martinfleis for working on this! Having docs available is a great addition; it also highlights some of the areas where our docstrings need a bit of work. :)
I'm not very familiar with setting up Sphinx, so I'm not able to provide much feedback about that here other than that it appears to be working properly in most cases.
It looks like perhaps some of our docstrings are triggering weird formatting? See the return statement here
- it shouldn't be highlighting the opening curly brace, right? (not that you should fix docstrings here, just wanted to flag in case it was a Sphinx config issue).
Do you see any easy ways to reduce the duplication of content between markdown files at the project root and markdown / RST files used for building docs? For instance, could we instead have a installing.md
at the project root and link to that from README (and drop that section from README), and then read from that when building docs, instead of having a dedicated docs/sourdce/install.md file? I'm not familiar enough with Sphinx to know what is easy / possible - just looking to centralize the updates we'd have to make in maintaining these...
docs/source/api.md
Outdated
@@ -0,0 +1,17 @@ | |||
# API reference |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think about using RST files as the standard under docs/source
similar to GeoPandas? I don't have a strong preference, but it seems like this would sidestep the need to add the {eval-rst}
context here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personally I like moving to markdown as much as possible, but indeed for files like this that is not very nice.
It should also be possible to only have this file in rst, to consider as another option.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have changed to have this one a pure RST. Since there's no text it doesn't matter so much.
Yes, that's because of the formatting (the way the dictionary with curly braces is formatted now gives unexpected whitespace/indentation for text interpreted as rst (not as code)). I think doing something like the following should fix that:
|
Another option could also be remove some content from the README, and have the README simply mostly refer to the docs? (personally I find this a bit trade-off as well, as I actually like have some nice content in the README because I typically go to a github repo to check something about a package, and then having to go to the docs is another indirection. But for most users that's maybe not an argument) |
I usually prefer succinct ReadMe with details in the docs. As the project grows, documentation gives you more options on how to format and split the information. So in the long term, I would move everything from ReadMe to docs and keep it much shorter, similarly to what we have in geopandas, xyzservices or contextily. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great - thank you so much @martinfleis ! 🎉
We can update the note about availability on PyPI (i.e., as a source distribution) in docs / readme after this is merged.
Setting up basic API documentation hosted on ReadTheDocs.