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

version collisions on readthedocs #3567

Closed
keewis opened this issue Nov 24, 2019 · 3 comments · Fixed by #3589
Closed

version collisions on readthedocs #3567

keewis opened this issue Nov 24, 2019 · 3 comments · Fixed by #3589

Comments

@keewis
Copy link
Collaborator

keewis commented Nov 24, 2019

In #3199 the documentation on readthedocs did not build because the newest packaged version of xarray was pulled in by a dependency -- in this case cfgrib. I'm not sure why, but this shadows the versions installed by at least python setup.py install --force and python -m pip install -e . (not python -m pip install ., I think, but readthedocs uses --update-strategy=eager for pip-installing, which deactivates version pinning).

Fortunately, cfgrib does not have a hard dependency on xarray (it's in the extras_require section), so this issue was bypassed in #3557 by using pip to install it. Should we ever want to introduce a dependency that requires xarray (or to use conda to install cfgrib), this is bound to resurface.

It might be that this is a problem with how versioneer constructs versions based on git commit hashes and a fix to #2853 is a fix to this, but it certainly needs more investigation.

#3369 is related, but more about catching these sort of issues before merging.

@keewis
Copy link
Collaborator Author

keewis commented Nov 27, 2019

this is something that looks somewhat like a hack to me, but how about not installing at all but rather using

PYTHONPATH=$(readlink -f ..) sphinx-build -M html -d _build/doctrees -n . _build/html

to call sphinx (the important part is the environment variable)? I think that should run the correct module even if a different version should happen to be installed.

Edit: unfortunately readlink -f .. (or something similar to get the absolute path) is required: just using .. is not possible. Unless the RTD build always puts the checked out project in the same location in the container, we need to execute a shell command.

Edit2: it might have side-effects, but using PYTHONPATH=..:../.. seems to work. The reason for this is that the kernels get started with PWD=doc/examples. Of course, that does not work in general (probably has the same issues as installing) and using the absolute path would be the correct way. Unfortunately, readthedocs puts a branch component into the directory path, which means we can't hard-code.

Actually, we only need PYTHONPATH=../.. since we can just modify sys.path in conf.py and also remove any side-effects:

import pathlib
import sys

root = pathlib.Path(__file__).parent.parent.absolute()
sys.path = [path for path in sys.path if path not in (str(root), str(root.parent))]
sys.path.insert(0, str(root))

but this looks even more like a hack to me. What do you think?

@keewis
Copy link
Collaborator Author

keewis commented Dec 3, 2019

It seems we can solve this by setting setup_py_install: false and adding these lines to conf.py:

import os
import pathlib
import sys

root = pathlib.Path(__file__).parent.parent.absolute()
os.environ["PYTHONPATH"] = str(root)
sys.path.insert(0, str(root))

what confuses me is that the nbsphinx documentation does not mention this trick.

Edit: I asked the nbsphinx developers about this, let's see what they think.

@keewis
Copy link
Collaborator Author

keewis commented Dec 6, 2019

looks like it is supported but the maintainer of nbsphinx didn't know about it.

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

Successfully merging a pull request may close this issue.

1 participant