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

Apigen recursive #2243

Merged
merged 14 commits into from
Mar 11, 2020
Merged

Apigen recursive #2243

merged 14 commits into from
Mar 11, 2020

Conversation

emmanuelle
Copy link
Contributor

still WIP, and it's been some time since I worked on this. Basically the idea is to call sphinx-apidoc to generate all the doc pages.

@@ -2251,8 +2251,8 @@
"dev": true,
"optional": true,
"requires": {
"co": "^4.6.0",
"json-stable-stringify": "^1.0.1"
"co": "4.6.0",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the changes to this file probably should be reverted.

@emmanuelle
Copy link
Contributor Author

@jonmmease, one thing I can't get my head around is which part of the codegen generates the docstrings of the @property methods, for example https://github.com/plotly/plotly.py/blob/master/packages/python/plotly/plotly/graph_objs/bar/hoverlabel/__init__.py#L67. For the docstrings of the constructors I found how to hack them to add the :py:func: directives creating internal links with sphinx, but not for the @property methods.

@emmanuelle
Copy link
Contributor Author

The doc build is a bit long (a few minutes), but the build is stored in the artifacts of CircleCI if you want to take a look.

@jonmmease
Copy link
Contributor

@emmanuelle, the property docstrings are generated dynamically during codegen by Validator instances. Check out the description method of the validator classes in packages/python/plotly/_plotly_utils/basevalidators.py. Does that help?

@emmanuelle
Copy link
Contributor Author

@jonmmease absolutely! I was grepping only the codegen directory, I should have known better :-). Thanks a lot!

@nicolaskruchten
Copy link
Contributor

So with this PR we get these extra directives in the docstrings that people will see when they try help(object) in a notebook?

@emmanuelle emmanuelle changed the title [WIP] Apigen recursive Apigen recursive Mar 10, 2020
@emmanuelle
Copy link
Contributor Author

@nicolaskruchten yes we'll have the extra directives. However I found out that we can use :class: instead of :py:class: which is a bit shorter :-).

@nicolaskruchten
Copy link
Contributor

I can live with :class in the docstrings... @jonmmease objections?

@emmanuelle
Copy link
Contributor Author

I could also attach a helper function to autodoc-process-docstring as described in https://www.sphinx-doc.org/en/master/usage/extensions/autodoc.html or https://chromium.googlesource.com/infra/third_party/sphinxcontrib_napoleon/+/refs/heads/master/sphinxcontrib/napoleon/__init__.py#253 in order to prepend the :class: to plotly.graph_objects.xx strings. But this can also be done later on :-).

@emmanuelle
Copy link
Contributor Author

Ready for review!

@@ -472,6 +472,7 @@ jobs:
git config user.name plotlydocbot
git config user.email accounts@plot.ly
git add *
git add *
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

redundant?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hum let me fix this

@nicolaskruchten
Copy link
Contributor

💃 ! This is great :)

@emmanuelle emmanuelle merged commit 30cad0c into master Mar 11, 2020
@emmanuelle emmanuelle deleted the apigen-recursive branch March 27, 2020 16:28
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.

3 participants