Skip to content

[SPARK-24530][PYTHON] Add a control to force Python version in Sphinx via environment variable, SPHINXPYTHON #21659

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.

Sign up for GitHub

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

Closed
HyukjinKwon wants to merge 8 commits into from
Closed

[SPARK-24530][PYTHON] Add a control to force Python version in Sphinx via environment variable, SPHINXPYTHON #21659

Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
a1346e2
Add a control to force Python version via enviornment variable
HyukjinKwon Jun 28, 2018
dd303b6
Python change to trigger the stlye check
HyukjinKwon Jun 29, 2018
69f31a9
Revert "Python change to trigger the stlye check"
HyukjinKwon Jun 29, 2018
135d613
Address comments and correct its priority
HyukjinKwon Jul 2, 2018
71ff040
Fix some comments
HyukjinKwon Jul 2, 2018
950ead0
Run pydocstyle
HyukjinKwon Jul 9, 2018
d500e0d
Revert "Run pydocstyle"
HyukjinKwon Jul 9, 2018
2e1e7da
Remove changes in README.md which makes backporting harder
HyukjinKwon Jul 10, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Fix some comments
  • Loading branch information
@HyukjinKwon
HyukjinKwon committed Jul 2, 2018
commit 71ff04080c716b32dd46e3a81fa3922e489ce30c
4 changes: 2 additions & 2 deletions python/docs/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ endif
endif

ifdef SPHINXBUILD
Copy link
Contributor

@holdenk holdenk Jul 3, 2018

Choose a reason for hiding this comment

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

Is there a way to check that Sphinx build is running with Py3 as done bellow?

Copy link
Member Author

@HyukjinKwon HyukjinKwon Jul 4, 2018

Choose a reason for hiding this comment

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

I couldn't find an easy way.

# User-friendly check for sphinx-build if explicitly specified.
# User-friendly check for sphinx-build.
ifeq ($(shell which $(SPHINXBUILD) >/dev/null 2>&1; echo $$?), 1)
$(error The '$(SPHINXBUILD)' command was not found. Make sure you have Sphinx installed, then set the SPHINXBUILD environment variable to point to the full path of the '$(SPHINXBUILD)' executable. Alternatively you can add the directory with the executable to your PATH. If you don't have Sphinx installed, grab it from http://sphinx-doc.org/)
endif
Copy link
Member

@viirya viirya Jun 29, 2018

Choose a reason for hiding this comment

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

If SPHINXPYTHON -msphinx' is preferred instead of 'sphinx-build' by default, should we go for a default SPHINXPYTHON + -msphinx instead of a default SPHINXBUILD if both are not set?

Copy link
Member Author

@HyukjinKwon HyukjinKwon Jun 29, 2018

Choose a reason for hiding this comment

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

Yea... that was the problem I faced.. Probably we should change but seems it needs some changes in Jenkins environment .. FYI there are some discussion about the potential effect by setting that as a default in the PR link of the comments.

I was thinking we set SPHINXPYTHON manually when we make a release for now.

Copy link
Contributor

@holdenk holdenk Jul 2, 2018

Choose a reason for hiding this comment

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

That seems error prone - why not pick a good default of python3 and allow overriding?

Expand All @@ -23,7 +23,7 @@ endif
ifeq ($(shell $(SPHINXPYTHON) -c 'import sphinx' >/dev/null 2>&1; echo $$?), 1)
$(error Python executable '$(SPHINXPYTHON)' did not have Sphinx installed. Make sure you have Sphinx installed, then set the SPHINXPYTHON environment variable to point to the Python executable having Sphinx installed. If you don't have Sphinx installed, grab it from http://sphinx-doc.org/)
endif
# Use 'SPHINXPYTHON -msphinx' instead of 'sphinx-build' by default. See https://github.com/sphinx-doc/sphinx/pull/3523 for more details.
# Use 'SPHINXPYTHON -msphinx' instead of 'sphinx-build'. See https://github.com/sphinx-doc/sphinx/pull/3523 for more details.
SPHINXBUILD = $(SPHINXPYTHON) -msphinx
endif

Expand Down