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

python: add sphinx docs #6525

Merged
merged 5 commits into from Feb 11, 2020
Merged

Conversation

@tswast
Copy link
Contributor

tswast commented Aug 15, 2019

Background:

Formerly, the Python protobuf reference documentation was built with
Epydoc. This package has not been
updated since 2008, and it has inconsistent formatting (see internal
issue 131415575) with most Python documentation. Sphinx is used for the
official docs.python.org docs as well as most other Python packages,
including the Google client libraries and related packages, such as
https://googleapis.dev/python/google-api-core/latest/

To build the docs with Sphinx:

  1. Install the needed packages (sphinx, sphinxcontrib-napoleon for
    Google-style docstring support). I've created a conda environment file
    to make this easier:
conda env create -f python/docs/environment.yml
  1. (Optional) Generate reference docs files and regenerate index:
cd python
python generate_docs.py
cd ..
  1. Run Sphinx.
cd python/docs
make html

About this change:

The script at python/generate_docs.py creates a ReStructured Text file
for each public module in the protobuf Python package. The script also
updates the table of contents in python/docs/index.rst to point to
these module references.

Future work:

Testing the docs build on PRs requires contributors to actually do some
setup work to configure builds on their fork. It'd be better if CI had a
docs build session to verify that the Sphinx docs generation at least
runs.

There are many warnings due to not-quite-correct docstrings in the
actual Python code itself. I'm choosing to ignore these errors to keep
the PR small, but I recommend you fix these and then enable "fail on
warnings" in the docs build on CI.

Towards #4498

@googlebot googlebot added the cla: yes label Aug 15, 2019
@tswast tswast force-pushed the tswast:issue4498-sphinx branch from cfc769e to c964f44 Jan 14, 2020
@tswast tswast force-pushed the tswast:issue4498-sphinx branch from 7d58f7b to 75e6ed1 Jan 22, 2020
@tswast tswast changed the title WIP: Sphinx docs add sphinx docs, built by readthedocs Jan 22, 2020
@tswast tswast marked this pull request as ready for review Jan 22, 2020
@tswast

This comment has been minimized.

Copy link
Contributor Author

tswast commented Jan 22, 2020

@tswast tswast force-pushed the tswast:issue4498-sphinx branch 2 times, most recently from 639e9eb to a92e3d8 Jan 23, 2020
@anandolee anandolee self-requested a review Jan 24, 2020
@tswast tswast force-pushed the tswast:issue4498-sphinx branch 2 times, most recently from 9d1ffc6 to 1e7cf88 Jan 27, 2020
@tswast tswast changed the title add sphinx docs, built by readthedocs python: add sphinx docs Jan 29, 2020
@tswast

This comment has been minimized.

Copy link
Contributor Author

tswast commented Jan 29, 2020

I pulled the Read the Docs config into a separate PR at #7150 (relevant commit: e836508)

@tswast tswast force-pushed the tswast:issue4498-sphinx branch from 1e7cf88 to b9af133 Jan 30, 2020
@tswast tswast force-pushed the tswast:issue4498-sphinx branch from b9af133 to da5cba4 Jan 31, 2020
@tswast

This comment has been minimized.

Copy link
Contributor Author

tswast commented Feb 5, 2020

What's this EXTRA_DIST in the test failures?

I'm guessing that I need to list all files and add them here:

python_EXTRA_DIST= \

Edit: Done in 04104a3

tswast added 2 commits Aug 15, 2019
Background:

Formerly, the Python protobuf reference documentation was built with
[Epydoc](http://epydoc.sourceforge.net/). This package has not been
updated since 2008, and it has inconsistent formatting (see internal
issue 131415575) with most Python documentation. Sphinx is used for the
official docs.python.org docs as well as most other Python packages,
including the Google client libraries and related packages, such as
https://googleapis.dev/python/google-api-core/latest/

To build the docs with Sphinx:

1. Install the needed packages (`sphinx`, `sphinxcontrib-napoleon` for
Google-style docstring support). I've created a conda environment file
to make this easier:

```
conda env create -f python/docs/environment.yml
```

2. (Optional) Generate reference docs files and regenerate index:

```
cd python
python generate_docs.py
cd ..
```

3. Run Sphinx.

```
cd python/docs
make html
```

About this change:

The script at `python/generate_docs.py` creates a ReStructured Text file
for each public module in the protobuf Python package. The script also
updates the table of contents in `python/docs/index.rst` to point to
these module references.

Future work:

Testing the docs build on PRs requires contributors to actually do some
setup work to configure builds on their fork. It'd be better if CI had a
docs build session to verify that the Sphinx docs generation at least
runs.

There are many warnings due to not-quite-correct docstrings in the
actual Python code itself. I'm choosing to ignore these errors to keep
the PR small, but I recommend you fix these and then enable "fail on
warnings" in the docs build on CI.
@tswast tswast force-pushed the tswast:issue4498-sphinx branch from da5cba4 to 04104a3 Feb 5, 2020
@anandolee anandolee added the kokoro:run label Feb 5, 2020
# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

This comment has been minimized.

Copy link
@anandolee

anandolee Feb 5, 2020

Contributor

Maybe also add the generate instructions (which is in the RP description) in this file

This comment has been minimized.

Copy link
@tswast

tswast Feb 5, 2020

Author Contributor

done in df72847

@tswast tswast requested a review from anandolee Feb 5, 2020
Makefile.am Outdated
@@ -915,6 +915,36 @@ php_EXTRA_DIST= \

python_EXTRA_DIST= \
python/MANIFEST.in \
python/generate_docs.py \

This comment has been minimized.

Copy link
@anandolee

anandolee Feb 6, 2020

Contributor

Move those files to nodist_... (maybe nodist_python_doc) instead of python_EXTRA_DIST. Because we do not want to add these files into release package.

Examples of other not released files that use nodist_
https://github.com/protocolbuffers/protobuf/search?q=nodist&unscoped_q=nodist

Or I am wondering if it is better to move this tool to google3 (google3/third_party/protobuf/testing/protobuf/python/)?

This comment has been minimized.

Copy link
@tswast

tswast Feb 6, 2020

Author Contributor

I tried adding:


nodist_python_docs_SOURCES=                                                  \
  python/generate_docs.py                                                    \
  python/docs/requirements.txt                                               \
  python/docs/make.bat                                                       \
  python/docs/index.rst                                                      \
  python/docs/google/protobuf/wrappers_pb2.rst                               \
  python/docs/google/protobuf/type_pb2.rst                                   \
  python/docs/google/protobuf/timestamp_pb2.rst                              \
  python/docs/google/protobuf/text_format.rst                                \
  python/docs/google/protobuf/text_encoding.rst                              \
  python/docs/google/protobuf/symbol_database.rst                            \
  python/docs/google/protobuf/struct_pb2.rst                                 \
  python/docs/google/protobuf/service_reflection.rst                         \
  python/docs/google/protobuf/service.rst                                    \
  python/docs/google/protobuf/reflection.rst                                 \
  python/docs/google/protobuf/proto_builder.rst                              \
  python/docs/google/protobuf/message_factory.rst                            \
  python/docs/google/protobuf/message.rst                                    \
  python/docs/google/protobuf/json_format.rst                                \
  python/docs/google/protobuf/field_mask_pb2.rst                             \
  python/docs/google/protobuf/empty_pb2.rst                                  \
  python/docs/google/protobuf/duration_pb2.rst                               \
  python/docs/google/protobuf/descriptor_pool.rst                            \
  python/docs/google/protobuf/descriptor_pb2.rst                             \
  python/docs/google/protobuf/descriptor_database.rst                        \
  python/docs/google/protobuf/descriptor.rst                                 \
  python/docs/google/protobuf/any_pb2.rst                                    \
  python/docs/google/protobuf.rst                                            \
  python/docs/environment.yml                                                \
  python/docs/conf.py                                                        \
  python/docs/Makefile

but it has no effect on build_cpp_distcheck. Instead, in 30763ff I've updated tests.sh to exclude the docs directory and moved the generate_docs.py script there.

Or I am wondering if it is better to move this tool to google3 (google3/third_party/protobuf/testing/protobuf/python/)?

Which tool?

Move the sphinx config to internal? Most open source packages provide a way to build the docs for the package. Having the docs here would encourage contributions to the docs, which can be an easier way to get started with contributing to a project than diving into the code.

Or move the python_EXTRA_DIST stuff? It wasn't documented in the CONTRIBUTING file, but after a bit of digging, I was able to figure out how that I could install libtool and automake and run ./tests.sh cpp_distcheck.

I had to switch to Python 2 due to

"./scripts/fuse_gmock_files.py" "./fused-src"
  File "./scripts/fuse_gmock_files.py", line 235
    print __doc__
                ^
SyntaxError: Missing parentheses in call to 'print'. Did you mean print(__doc__)?

This comment has been minimized.

Copy link
@anandolee

anandolee Feb 7, 2020

Contributor

Yes I was wondering Move the sphinx config to internal. But it sounds reasonable to be here as you mentioned it would encourage contributions

@tswast tswast requested a review from anandolee Feb 7, 2020
@anandolee anandolee added the kokoro:run label Feb 7, 2020
@anandolee

This comment has been minimized.

Copy link
Contributor

anandolee commented Feb 8, 2020

our kokoro is breaking by another PR. It will be fixed soon. Let's wait a few hours to rerun the tests

@anandolee anandolee added the kokoro:run label Feb 8, 2020
…ue4498-sphinx
@tswast

This comment has been minimized.

Copy link
Contributor Author

tswast commented Feb 10, 2020

Just merged with upstream/master, hopefully that get the kokoro fix.

@tswast

This comment has been minimized.

Copy link
Contributor Author

tswast commented Feb 11, 2020

Not sure what all these

Error response from daemon: unexpected EOF

errors in Kokoro are all about.

@anandolee

This comment has been minimized.

Copy link
Contributor

anandolee commented Feb 11, 2020

@anandolee anandolee merged commit 29c83ba into protocolbuffers:master Feb 11, 2020
43 of 56 checks passed
43 of 56 checks passed
Linux Golang Kokoro build failed
Details
Linux Java Compatibility Kokoro build failed
Details
Linux Python 2.7 Kokoro build failed
Details
Linux Python 2.7 C++ Kokoro build failed
Details
Linux Python 3.3 Kokoro build failed
Details
Linux Python 3.3 C++ Kokoro build failed
Details
Linux Python 3.4 Kokoro build failed
Details
Linux Python 3.4 C++ Kokoro build failed
Details
Linux Python 3.7 Kokoro build failed
Details
Linux Python 3.7 C++ Kokoro build failed
Details
Linux Python 3.8 C++ Kokoro build failed
Details
Linux Python C++ Kokoro build failed
Details
MacOS Python Release Kokoro build failed
Details
Bazel Kokoro build successful
Details
Dist artifact installation Kokoro build successful
Details
Linux 32-bit Kokoro build successful
Details
Linux C# Kokoro build successful
Details
Linux C++ Distcheck Kokoro build successful
Details
Linux C++ TC Malloc Kokoro build successful
Details
Linux Java JDK 7 Kokoro build successful
Details
Linux Java Linkage Monitor Kokoro build successful
Details
Linux Java Oracle 7 Kokoro build successful
Details
Linux JavaScript Kokoro build successful
Details
Linux PHP Kokoro build successful
Details
Linux Python Kokoro build successful
Details
Linux Python 3.5 Kokoro build successful
Details
Linux Python 3.5 C++ Kokoro build successful
Details
Linux Python 3.6 Kokoro build successful
Details
Linux Python 3.6 C++ Kokoro build successful
Details
Linux Python 3.8 Kokoro build successful
Details
Linux Python Compatibility Kokoro build successful
Details
Linux Python Release Kokoro build successful
Details
Linux Ruby 2.4 Kokoro build successful
Details
Linux Ruby 2.5 Kokoro build successful
Details
Linux Ruby 2.6 Kokoro build successful
Details
Linux Ruby Release Kokoro build successful
Details
MacOS C++ Kokoro build successful
Details
MacOS C++ Distcheck Kokoro build successful
Details
MacOS JavaScript Kokoro build successful
Details
MacOS Obj-C CocoaPods Integration Kokoro build successful
Details
MacOS Obj-C OS X Kokoro build successful
Details
MacOS Obj-C iOS Debug Kokoro build successful
Details
MacOS Obj-C iOS Release Kokoro build successful
Details
MacOS PHP5.6 Kokoro build successful
Details
MacOS PHP7.0 Kokoro build successful
Details
MacOS Python Kokoro build successful
Details
MacOS Python C++ Kokoro build successful
Details
MacOS Ruby 2.4 Kokoro build successful
Details
MacOS Ruby 2.5 Kokoro build successful
Details
MacOS Ruby 2.6 Kokoro build successful
Details
MacOS Ruby Release Kokoro build successful
Details
Mergeable Mergeable Run has been Completed!
Details
Windows C# Kokoro build successful
Details
Windows Csharp Release Kokoro build successful
Details
Windows Python Release Kokoro build successful
Details
cla/google All necessary CLAs are signed
@tswast tswast deleted the tswast:issue4498-sphinx branch Feb 11, 2020
@tswast

This comment has been minimized.

Copy link
Contributor Author

tswast commented Feb 11, 2020

Thanks! I'll rebase #7154 soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.