[FLINK-12720][python] Add the Python Table API Sphinx docs #8732
Conversation
|
Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community Review Progress
Please see the Pull Request Review Guide for a full explanation of the review process. DetailsThe Bot is tracking the review progress through labels. Labels are applied according to the order of the review items. For consensus, approval by a Flink committer of PMC member is required Bot commandsThe @flinkbot bot supports the following commands:
|
There was a problem hiding this comment.
@HuangXingBo Thanks a lot for the PR. Great work! I have left a few minor comments. Besides, the following issue should also be addressed: Some target of the Makefile will fail when executed such as make doctest, make pdf, etc.
flink-python/docs/index.rst
Outdated
|
|
||
| :class:`pyflink.table.TableEnvironment` | ||
|
|
||
| Main entry point for Flink functionality. |
There was a problem hiding this comment.
What about Main entry point for Flink Table functionality.
| :maxdepth: 1 | ||
|
|
||
| pyflink.table | ||
|
|
There was a problem hiding this comment.
Add the following text here to indicate that the following classes are the content of package pyflink:
| Contents | |
| -------- |
| .. automodule:: pyflink.table | ||
| :members: | ||
| :undoc-members: | ||
| :show-inheritance: |
There was a problem hiding this comment.
One empty line at the end of the file
sunjincheng121
left a comment
sunjincheng121
left a comment
There was a problem hiding this comment.
Thanks for the PR @HuangXingBo! Great Job!
I one left one comment, and a suggestion is we should copy the python doc to docs content folder, and I am not sure, but I guess that the Apache CI will auto ren the Travis, and copy the content to https://ci.apache.org/projects/flink/flink-docs-master. And we also should add the link for Python Docs, just like: https://ci.apache.org/projects/flink/flink-docs-master/api/java/. for the Python Docs, maybe `https://ci.apache.org/projects/flink/flink-docs-master/api/python. So, we may need to add a plugin for Jekyll.
What do you think?
flink-python/docs/conf.py
Outdated
| author = u'Author' | ||
|
|
||
| # The short X.Y version | ||
| version = '1.0' |
There was a problem hiding this comment.
The version should same as : https://github.com/apache/flink/blob/master/flink-python/pyflink/version.py
|
I check it again, I found that Java and Scala Doc are built by BuilderBot(https://ci.apache.org/projects/flink/), The BuilderBot need a config |
|
Thanks a lot for @dianfu @sunjincheng121.
|
dianfu
left a comment
dianfu
left a comment
There was a problem hiding this comment.
@HuangXingBo Thanks a lot for the update. Mostly LGTM. Just left a few comments regarding to the newly added script generate-docs.sh.
| @@ -0,0 +1,53 @@ | |||
| ::############################################################################### | |||
| :: Licensed to the Apache Software Foundation (ASF) under one | |||
There was a problem hiding this comment.
It seems that this script still does not work? We can remove it for now if the windows support is not ready.
| echo "Copy python api docs to $TARGET_PATH failed." | ||
| exit 1 | ||
| fi | ||
|
|
There was a problem hiding this comment.
one empty line at the end of file is enough
| _BUILD_PATH="./_build" | ||
|
|
||
| if [[ ! -d "$_BUILD_PATH" ]]; then | ||
| echo "The directory $_BUILD_PATH is not created.you can retry to this script" |
There was a problem hiding this comment.
- empty space before the word
you - retry to -> retry to execute
|
|
||
| _HTML_PATH="${_BUILD_PATH}/html" | ||
| if [[ ! -d "$_HTML_PATH" ]]; then | ||
| echo "The directory $_HTML_PATH is not created.you can retry to this script" |
There was a problem hiding this comment.
- empty space before the word
you - retry to -> retry to execute
| fi | ||
|
|
||
| if [[ ! -d "$TARGET_PATH" ]]; then | ||
| echo "The supported dir is not exist." |
There was a problem hiding this comment.
The specified directory does not exist.
| USAGE=" | ||
| usage: $0 [options] | ||
| -h print this help message and exit | ||
| -t list all checks supported. |
There was a problem hiding this comment.
The description for option -t seems not correct.
| esac | ||
| done | ||
|
|
||
| LINTPYTHON_PATH="../dev/lint-python.sh" |
There was a problem hiding this comment.
The means that we must execute this script under directory doc.
What about changed as follows:
FLINK_PYTHON_DOCS_DIR="$(cd "$( dirname "$0" )" && pwd)"
LINTPYTHON_PATH="$FLINK_PYTHON_DOCS_DIR/../dev/lint-python.sh"
|
@dianfu Thanks for helping improve the PR! @HuangXingBo told me he is on the vacation cannot update the PR those days, he also agrees that if you want to help him, you can open a new PR with @HuangXingBo 's commit as a basic commit. What do you think? |
|
@sunjincheng121 Thanks a lot for the suggestion. Makes sense to me and I have created PR #8774 based on @HuangXingBo 's great work. |
|
Thanks for your effort @HuangXingBo! and Thanks for open the new PR @dianfu! |
5 participants
What is the purpose of the change
As the Python Table API is added, we should add the Python Table API Sphinx docs. This includes the following work:
Add scripts to build the Sphinx docs
Brief change log
Verifying this change
This change is a trivial rework / code cleanup without any test coverage.
Does this pull request potentially affect one of the following parts:
@Public(Evolving): (no)Documentation