-
Notifications
You must be signed in to change notification settings - Fork 38
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
Initial API docs config using Sphinx and readthedocs. #434
Conversation
docs/api_docs/core/aot.rst
Outdated
.. automodule:: iree.turbine.aot | ||
:members: | ||
:undoc-members: |
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.
@Groverkss noticed that https://github.com/iree-org/iree-turbine/blob/main/iree/turbine/aot/exporter.py is omitted here. That's pretty critical to include in the docs. Can try adding back :imported-members:
here, or add a dedicated section for that file.
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.
Added export back with a few tweaks to the .rst:
https://iree-turbine-scotttoddtest.readthedocs.io/en/latest/core/aot.html#iree.turbine.aot.export
https://iree-turbine-scotttoddtest.readthedocs.io/en/latest/core/aot.html#iree.turbine.aot.ExportOutput
It probably makes more logical sense to avoid :imported-members:
and group by file instead of alphabetically sorting every member that gets imported by this though:
iree-turbine/iree/turbine/aot/__init__.py
Lines 11 to 17 in 34387e7
from .builtins import * | |
from .compiled_module import * | |
from .decompositions import * | |
from .exporter import * | |
from .fx_programs import FxPrograms, FxProgramsBuilder | |
from .tensor_traits import * | |
from .params import * |
On my end, I want to get this website committed and hosted then let more local project developers maintain the docs. That is one area that can be improved.
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! What do you think about skipping api_docs
and moving it all to docs
instead?
docs/api_docs/.readthedocs.yaml
Outdated
@@ -0,0 +1,17 @@ | |||
version: 2 |
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 about having this config in the root of the repository (default) or a level up under docs
?
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 like keeping a separation between markdown documentation meant for viewing on GitHub and source and configuration files for a website. Turbine only has one markdown doc right now and it is directed at project maintainers, not project users (https://github.com/iree-org/iree-turbine/blob/main/docs/releasing.md). I guess I could merge that into the "api docs", though I do eventually want to merge more of our website though and I'm not sure how that would fit in.
- https://iree.dev/
- https://iree-python-api.readthedocs.io/
- https://iree-turbine.readthedocs.io/
- https://shortfin.readthedocs.io/
- https://github.com/nod-ai/shark-ai/tree/main/docs
If the docs are kept separate, this yaml file could be moved up a level or two, but I'd rather keep it next to the other sphinx and website config files (conf.py, requirements.txt, etc.)
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.
Merged docs/
with docs/api_docs/
. What do you think about this setup? (demo still hosted at https://iree-turbine-scotttoddtest.readthedocs.io/en/latest/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.
Looks good to me. We could think about swap Project documentation
and API reference material
but we can iterate and sort this out later.
Progress on #77. There is still plenty to iterate on, but this gets something started that developers can start using and improving.
Important
A demo website is hosted at https://iree-turbine-scotttoddtest.readthedocs.io/en/latest to help with review.
I also claimed https://app.readthedocs.org/projects/iree-turbine/ and will configure it once this is merged. The final hosted site will be at https://iree-turbine.readthedocs.io/en/latest/ and will update after each commit to the
main
branch in this repository.Notes
I've structured the site into a few main sections:
Some module setups are conflicting with how
autodoc
wants modules to be defined. I worked with what we have now, but we may want to change how modules are defined and imports are used. I'm fairly inexperienced at this level, but I think a reliance on relative imports like these is leading to packages that can't easily be used in isolation:iree-turbine/iree/turbine/kernel/wave/codegen.py
Lines 83 to 94 in a7c3332
This show up in the docs where something like
automodule:: iree.turbine.aot.builtins
for a given module is missing significant classes and functions and code paths like https://github.com/iree-org/iree-turbine/tree/main/iree/turbine/kernel/compiler have no__init__.py
to create a package.