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

first round of changes towards doctesting #1921

Merged
merged 18 commits into from
Nov 29, 2019
Merged

first round of changes towards doctesting #1921

merged 18 commits into from
Nov 29, 2019

Conversation

emmanuelle
Copy link
Contributor

Not a full doctest workflow yet, but some required changes.

@emmanuelle
Copy link
Contributor Author

Also see #1919 and #1920.

@@ -161,91 +161,9 @@ def create_ohlc(open, high, low, close, dates=None, direction="both", **kwargs):
>>> from plotly.figure_factory import create_ohlc
>>> from datetime import datetime

>>> import pandas.io.data as web
Copy link
Contributor Author

Choose a reason for hiding this comment

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

here I removed all of them because the doc was broken and we have the trace anyway

@@ -468,8 +469,9 @@ def update(self, dict1=None, overwrite=False, **kwargs):
>>> fig = go.Figure(layout={'xaxis':
... {'color': 'green',
... 'range': [0, 1]}})
>>> fig.update({'layout': {'xaxis': {'color': 'pink'}}})
>>> fig.to_plotly_json()
>>> fig.update({'layout': {'xaxis': {'color': 'pink'}}}) #doctest: +ELLIPSIS
Copy link
Contributor

Choose a reason for hiding this comment

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

another approach would be fig = fig.update( ... ) ?

@emmanuelle
Copy link
Contributor Author

Note that sphinx removes the doctest flags
image

@nicolaskruchten
Copy link
Contributor

Cool :)

@nicolaskruchten
Copy link
Contributor

All seems pretty good to me! I notice that Sphinx grabs the first sentence or line of docstrings to make these "TOC" type pages: https://plot.ly/python-api-reference/plotly.figure_factory.html so we'll want to take a look at this page with these new docstrings to make sure we're happy with it.

Maybe the deprecated ones all start with **deprecated** and show up at the end or something?

@emmanuelle
Copy link
Contributor Author

So here is how the table of contents looks like
image
For figure factories replaced by a trace I just wrote deprecated at the start. For some other functions it's not a 1-1 correspondence and there is an example in the docstring, so I did not want to put off people from clicking on the link by having deprecated at the very start.

@nicolaskruchten
Copy link
Contributor

Looks good! Can we order them such that the non-deprecated ones are up top?

@emmanuelle
Copy link
Contributor Author

emmanuelle commented Nov 27, 2019

Sure! But this needs to be done in plotly.py-docs :-).

@emmanuelle
Copy link
Contributor Author

Yehaah, I think this one is ready for review @nicolaskruchten !

@emmanuelle emmanuelle mentioned this pull request Nov 28, 2019
@nicolaskruchten
Copy link
Contributor

Nice!

So (how) does this test the docstrings for e.g. go.Scatter ? this runs after codegen?

@nicolaskruchten
Copy link
Contributor

Ah, haha, there are no code examples in go.Scatter :)

@emmanuelle
Copy link
Contributor Author

Exactly :-). Actually we don't have a lot of docstrings with code examples. You can see a list of the files with docstrings doctested in the corresponding CI job (python3.7-orca)

@@ -36,7 +36,8 @@ def create_table(
**kwargs
):
"""
BETA function that creates data tables
**deprecated**, use instead the plotly.graph_objects trace
Copy link
Contributor

Choose a reason for hiding this comment

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

I actually realize now that this isn't an older way of making tables, but it's a convenience method for making tables, because go.Table is pretty hard to use. So maybe this one shouldn't be deprecated after all.

@nicolaskruchten
Copy link
Contributor

💃 modulo table comment. Thanks for this cleanup and lock-down!

@emmanuelle emmanuelle merged commit 07f8c69 into master Nov 29, 2019
@emmanuelle emmanuelle deleted the doctest-modules branch December 2, 2019 17:03
@nicolaskruchten nicolaskruchten added this to the v4.4.0 milestone Dec 10, 2019
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