ARROW-3834: [Doc] Merge C++ and Python documentation by xhochy · Pull Request #2856 · apache/arrow

xhochy · 2018-10-28T18:22:55Z

No description provided.

xhochy · 2018-10-28T18:29:25Z

@wesm @pitrou Would love to get your feedback on this early as it involves some work. The C++ part works nice with the XML generated by doxygen but one needs to explicitly list the parts that should show up in Sphinx. There is also an autoindex variant for C++ but that produces a wall of errors. I'm quite happy with the manual clustering approach. While it will lead to some classes not being automatically included in the documentation, it already gives the user a categorisation of the classes.

@kou As you mentioned in the mail thread, GLib needs the XML for GObject introspection to work, so I'm not going to touch that. I would still add top-level sections for the languages based on it and link to the generated API docs. Would that be ok for you as a first shot?

kou · 2018-10-29T00:17:49Z

Would that be ok for you as a first shot?

Yes!

pitrou · 2018-10-29T08:39:42Z

I almost never read the C++ docs (reading the .h files instead), so I'm not sure how people like to use it currently. I like this PR on the principle.

wesm · 2018-10-29T09:38:34Z

I will take a look when I can. We might go ahead and promote this to the top level of the repository to start having more documentation across languages

kszucs · 2018-11-01T14:30:28Z

I'm not sure about this. IMHO We should either move everything (including go, rust, java, ruby, etc.) to sphinx and make doc a top level directory or use the standard documentation tool for that particular language.

wesm · 2018-11-01T15:08:15Z

I think there's two kinds of documentation:

Prose explanations of things
Generated / browsable API documentation

In Python we are using Sphinx for both. Other projects only are doing the latter.

I would be in favor of having a top level doc/ directory where we develop prose documentation for the entire Arrow project including some of its programming language implementations. How we deal with generated API documentation may be on a case by case basis, but the doc index page should have links to every API listing that exists (Java, GLib, Python, Ruby, JavaScript, etc.). Getting every project's API docs into a single Sphinx project is probably too much to ask

pitrou · 2018-11-12T08:45:41Z

I wanted to give this a try but it seems doc building currently fails. The Python API docs are not generated anymore.

xhochy · 2018-11-12T08:53:33Z

@pitrou I will have a look later but one thing I noticed is that it always helped to have the Python package build and installed using both:

python setup.py build_ext --build-type=$ARROW_BUILD_TYPE --with-parquet --with-plasma --inplace
python setup.py build_ext --build-type=$ARROW_BUILD_TYPE --with-parquet --with-plasma install

xhochy · 2018-11-12T09:45:25Z

@pitrou rebased and also seeing a problem with the API docs.

xhochy · 2018-11-12T10:20:24Z

These were issues with sphinx>=1.8. I'll push a fix soon.

pitrou · 2018-11-12T18:21:52Z

The nice thing with this PR is that it would allow easy API cross-links from prose documentation, e.g.:

.. cpp:namespace:: arrow
Call :cpp:func:`Array::IsNull` to query a :cpp:class:`Array`'s validity bitmap.

One downside is that building docs becomes a bit more complicated (it also seems to make Sphinx building slower).

xhochy · 2018-11-20T21:41:13Z

@kszucs This now also contains a docker setup for building the Sphinx documentation.

@wesm @pitrou This is ready for review / merge. It contains a very basic set of C++ documentation. I would like to work on that in a separate PR and get the things here merged to prevent further merge conflicts.

pitrou · 2018-11-21T12:41:17Z

Apparently there's a failed Travis-CI entry.

pitrou · 2018-11-21T12:41:37Z

Use a wildcard for the version number here?

pitrou · 2018-11-21T12:42:02Z

What is recommonmark?

conda_env_cpp.yml and conda_env_python.yml files contain arrow-cpp's and pyarrow's dependencies and eventually will be used to spin up the development environment, see #2836
I suggest to create a ci/conda_env_sphinx.yml or conda_env_doc.yml with the sphinx dependencies, including doxygen and rsync too.
Of course this requires a minimal arrow/doc/Dockerfile too:
FROM arrow:python-3.6 # install python specific packagesADD ci/conda_env_sphinx.yml /arrow/ci/ RUN conda install -c conda-forge \ --file arrow/ci/conda_env_sphinx.yml && \ conda clean --all # build and testCMD arrow/ci/docker_build_cpp.sh && \ arrow/ci/docker_build_python.sh && \ arrow/ci/docker_build_sphinx.sh

pitrou · 2018-11-21T12:43:59Z

i'm -1 on including MarkDown docs in the Sphinx source tree. We should convert those documents to reST format. It's a bit of work, but it will be much nicer afterwards.

@wesm @jacques-n I cannot exactly recall but I think there were some strong voices that wanted the format docs to be in Markdown? I'm happy with converting them to reST format but only when people are fine with this.

I don't think anyone has ever proposed converting the format docs to another markup language. In principle I don't see an issue with using reST because people will mostly be consuming the documents via the generated webpage. These aren't documents that are going to be frequently edited, anyway

Ok, then I'll proceed and convert them to reST.

kszucs · 2018-11-21T12:59:03Z

Move to ci/conda_env_sphinx.yml?

kszucs · 2018-11-21T13:05:34Z

Please rename this file according to the Dockerfile example above: arrow/ci/docker_build_sphinx.sh and call these scripts from the docker CMD instead.

kszucs · 2018-11-21T13:08:21Z

There is an unused version variable in the conf.py which We should set to pyarrow.__version__ (suppose pyarrow is installed) and use here as html_title = u'Apache Arrow {}'.format(version)

kszucs · 2018-11-21T13:10:38Z

# Usage:# docker-compose build cpp# docker-compose build python# docker-compose build docs# docker-compose run docsimage: arrow:docsbuild: context: .dockerfile: doc/Dockerfile

kszucs

@xhochy I can reorganize the docker related stuff too if You don't have time for it.

Really minor: I'd use the plural docs instead of doc especially after merging multiple documentations under the same roof.

xhochy · 2018-11-29T20:21:01Z

@kszucs @pitrou Can you take a look at this again? I hope I have incorporated all comments.

pitrou · 2018-11-30T16:38:03Z

I'll take a look Monday.

wesm · 2018-12-04T03:28:28Z

I'm sorry to equivocate -- I rolled back this commit. I think that moving the protocol definitions from format/ to docs/source/format/ is incorrect. The protocol files (.fbs and .proto) should remain in format/ in the root of the repository, while the documentation about the format can be part of docs/. Does that sound reasonable?

xhochy · 2018-12-04T07:22:20Z

@wesm sounds reasonable. I'll move them

xhochy · 2018-12-05T17:37:57Z

@wesm Fixed, so this is ready for re-review.

wesm · 2018-12-06T00:29:00Z

Looks good. I will add a simple README to format/ and then merge this

Change-Id: I0be8ff87c6ffd352e9c26e01154dcf2da5743873

xhochy force-pushed the doc-merge branch from cd238b0 to 29484c5Compare November 12, 2018 09:43

xhochy force-pushed the doc-merge branch 2 times, most recently from f20e6d2 to ea08befCompare November 17, 2018 18:05

xhochy changed the title ~~RFC: Merge C++ and Python documentation~~ARROW-3834: [Doc] Merge C++ and Python documentationNov 18, 2018

pitrou requested changes Nov 21, 2018
View reviewed changes

kszucs reviewed Nov 21, 2018
View reviewed changes

Comment threadcpp/Dockerfile Outdated
kszucsNov 21, 2018
Copy link
Copy Markdown
Member
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move to ci/conda_env_sphinx.yml?

kszucs reviewed Nov 21, 2018
View reviewed changes

xhochy force-pushed the doc-merge branch 2 times, most recently from 147b354 to 5b6238bCompare November 27, 2018 21:45

xhochy force-pushed the doc-merge branch from 2c4edea to 896fa38Compare November 30, 2018 17:23

kszucs reviewed Dec 2, 2018
View reviewed changes

Comment threadpython/README.md Outdated

kszucs reviewed Dec 2, 2018
View reviewed changes

Comment threaddocs/source/python/pandas.rst Outdated

wesm deleted the doc-merge branch December 4, 2018 03:25

wesm restored the doc-merge branch December 4, 2018 03:27

wesm reopened this Dec 4, 2018

xhochyand others added 15 commits December 5, 2018 18:28

Merge C++ and Python documentation
9be6fbe

Fix Sphinx build for sphinx>=1.8
fd75660

Fix linter issues
d4cf542

Add breathe as a requirement
7850db8

Build C++ API docs
f7d5e92

Move Sphinx doc to top-level directory
985d428

Move doc to top-level
bdd824c

Fix references to format documents
671d244

Add pre-commit check for RAT
185cba8

Add doc generation to docker-compose
7aeff65

Convert format docs to reST
918e762

Rename doc to docs
0b4dd33

Fix Python docs build
78a5eaf

Add more C++ docs
fbe99c9

Review comments
337088b

xhochy mentioned this pull request Dec 5, 2018
ARROW-3943: [R] vignette #3107
Closed

Move format specifications back to /format/
071d16a

xhochy force-pushed the doc-merge branch from b2e6204 to 071d16aCompare December 5, 2018 17:37

Add simple README for the format/ directory
5b687ff
Change-Id: I0be8ff87c6ffd352e9c26e01154dcf2da5743873

wesm closed this in 35f8a34Dec 6, 2018

This was referenced Dec 6, 2018
[Doc] Merge Python & C++ and move to top-level #15877
Closed
[Python/Documentation] Include Benchmarks.md in Sphinx docs #20541
Closed

Uh oh!

Conversation

xhochy commented Oct 28, 2018

Uh oh!

xhochy commented Oct 28, 2018• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kou commented Oct 29, 2018

Uh oh!

pitrou commented Oct 29, 2018

Uh oh!

wesm commented Oct 29, 2018

Uh oh!

kszucs commented Nov 1, 2018• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wesm commented Nov 1, 2018

Uh oh!

pitrou commented Nov 12, 2018

Uh oh!

xhochy commented Nov 12, 2018

Uh oh!

xhochy commented Nov 12, 2018

Uh oh!

xhochy commented Nov 12, 2018

Uh oh!

pitrou commented Nov 12, 2018

Uh oh!

xhochy commented Nov 20, 2018

Uh oh!

pitrou commented Nov 21, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kszucsNov 21, 2018• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xhochyNov 21, 2018• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kszucsNov 21, 2018• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kszucs left a comment

Choose a reason for hiding this comment

Uh oh!

xhochy commented Nov 29, 2018

Uh oh!

pitrou commented Nov 30, 2018

Uh oh!

Uh oh!

Uh oh!

wesm commented Dec 4, 2018

Uh oh!

xhochy commented Dec 4, 2018

Uh oh!

xhochy commented Dec 5, 2018

Uh oh!

wesm commented Dec 6, 2018

Uh oh!

Reviewers

Assignees

Labels

xhochy commented Oct 28, 2018•
edited
Loading

kszucs commented Nov 1, 2018•
edited
Loading

kszucsNov 21, 2018•
edited
Loading

xhochyNov 21, 2018•
edited
Loading

kszucsNov 21, 2018•
edited
Loading