Graph objects structure and __all__ specifications

[jonmmease]

Code generation update to close #1799 and the graph_objects part of #1800.

@emmanuelle could you check if the placement of the __all__ statements under graph_objects helps sphinx find the classes to document?

[nicolaskruchten]

@emmanuelle in case this didn't get mentioned earlier, there's a CI step called dev_build which stores a pip-installable artifact that you can use to do stuff like check how Sphinx likes this PR. You can click on the dev_build CI step in the PR, then under "Artifacts" drill all the way down to the file called plotly-X.Y.Z-hash.tar.gz and right-click to get the URL and then you can pip install <url> locally.

Note that this build also includes the last master build of plotly.js, so it'll include stuff like go.Treemap today. This is very useful for doing plotly.js QA :)

[emmanuelle]

@jonmmease I pulled your branch and run the sphinx build of https://github.com/plotly/plotly.py-docs/pull/134/files. It does find the traces of graph_objects (hurray), the Figure class as well but not all its methods (I think the ones defined in the BaseFigure class are not found, such as update_traces, update_layout etc.). Also the Layout class is not found, I'm not sure why, maybe because of formatting issues (sphinx complains about Layout in the log). I'll investigate more but I wanted to share these first observations with you.

[emmanuelle]

Maybe it's easier for your diagnosis to pull plotly/plotly.py-docs#134 and to build the doc running make html inside the apidoc folder, the built page is _build/html/graph_objects.html.

[emmanuelle]

Also, I'll make a separate PR for the __all__ of other submodules (figure_factory, io).

[emmanuelle]

The Layout class is found, my mistake.

[emmanuelle]

Any reason why the traces in the __all__ are by inverse aphabetical order? I'd rather have them by alphabetical order, or we can also change the order in the sphinx page, like if we want Scatter and Bar to appear first.

[emmanuelle]

never mind for the order, I generated an alphabetical list in the sphinx PR.

[jonmmease]

Thanks for trying this out @emmanuelle. I'm not really sure what to do about sphinx not picking up the superclass methods. I wonder if there are sphinx configuration options for this.

[nicolaskruchten]

Does Sphinx or similar tools ever pick up superclass methods? In my experience with Javadoc etc I seem to recall this being the kind of thing that users have to click around to find... Here's an SO question about this: https://stackoverflow.com/questions/40508492/python-sphinx-inherit-method-documentation-from-superclass/40613306#40613306

[nicolaskruchten]

(I'd be OK with having a separate autodoc page for BaseFigure or even including BaseFigure in the same page as Figure if that works.)

[emmanuelle]

yes, we could have BaseFigure in the same page as Figure so that users can Ctrl-F.

[emmanuelle]

I noticed a problem with these changes though: when importing ddk in the same environment in get the error

----> 1 import plotly.graph_objs.layout

ModuleNotFoundError: No module named 'plotly.graph_objs.layout'; 'plotly.graph_objs' is not a package

no time to investigate right now, just wanted to report the issue

[jonmmease]

@emmanuelle oh that's interesting. Yeah, the import of the nested package plotly.graph_objs.layout isn't going to work if graph_objs is an alias to graph_objects. I guess ddk must perform the import that way.

Hmm, so to keep compatibility on this kind of import we won't be able to move the directory structure from graph_objs to graph_objects. For sphinx, do you think it would be enough to add the __all__ statement to the old graph_objects.py alias?

[nicolaskruchten]

Whew, nice catch Emma! Let’s try with just the __all__ piece I guess and see if we can fully move things later?

[emmanuelle]

@jonmmease @nicolaskruchten yes I think it will be ok just with the __all__. I'll check of course.

[jonmmease]

@emmanuelle, I updated the PR to only add the __all__ statements, but not move the graph_objs/ directory structure.

[emmanuelle]

Thank you @jonmmease I checked that the API doc looks good and finds all objects. So, 💃 !!

[jonmmease]

merging!