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

Allow access to __version__ with imports #746

Closed
wants to merge 1 commit into from
Closed

Conversation

michaeljones
Copy link
Collaborator

This is somewhat out of desperation. Not sure why this is failing on ReadTheDocs

We move the top level imports into the 'setup' function in order to
allow setup.py to access the version variable in the module without
having to have sphinx available.

I'm not sure if this is for the best but the ReadTheDocs build is
failing and it seems potentially related to this though I've no idea why
it is suddenly happening as we've not changed anything around this to my
understanding.

The ReadTheDocs error is:

/envs/latest/bin/python -m pip install --upgrade --upgrade-strategy eager --no-cache-dir .
Processing /home/docs/checkouts/readthedocs.org/user_builds/breathe/checkouts/latest
  DEPRECATION: A future pip version will change local packages to be built in-place without first copying to a temporary directory. We recommend you use --use-feature=in-tree-build to test your packages with this new behavior before it becomes the default.
   pip 21.3 will remove support for this functionality. You can find discussion regarding this at https://github.com/pypa/pip/issues/7555.
  Installing build dependencies: started
  Installing build dependencies: finished with status 'done'
  Getting requirements to build wheel: started
  Getting requirements to build wheel: finished with status 'error'
  ERROR: Command errored out with exit status 1:
   command: /home/docs/checkouts/readthedocs.org/user_builds/breathe/envs/latest/bin/python /home/docs/checkouts/readthedocs.org/user_builds/breathe/envs/latest/lib/python3.7/site-packages/pip/_vendor/pep517/in_process/_in_process.py get_requires_for_build_wheel /tmp/tmpo1quees5
       cwd: /tmp/pip-req-build-wrcudthy
  Complete output (26 lines):
  Traceback (most recent call last):
    File "/home/docs/checkouts/readthedocs.org/user_builds/breathe/envs/latest/lib/python3.7/site-packages/pip/_vendor/pep517/in_process/_in_process.py", line 349, in <module>
      main()
    File "/home/docs/checkouts/readthedocs.org/user_builds/breathe/envs/latest/lib/python3.7/site-packages/pip/_vendor/pep517/in_process/_in_process.py", line 331, in main
      json_out['return_val'] = hook(**hook_input['kwargs'])
    File "/home/docs/checkouts/readthedocs.org/user_builds/breathe/envs/latest/lib/python3.7/site-packages/pip/_vendor/pep517/in_process/_in_process.py", line 117, in get_requires_for_build_wheel
      return hook(config_settings)
    File "/tmp/pip-build-env-eaq7p8sq/overlay/lib/python3.7/site-packages/setuptools/build_meta.py", line 155, in get_requires_for_build_wheel
      config_settings, requirements=['wheel'])
    File "/tmp/pip-build-env-eaq7p8sq/overlay/lib/python3.7/site-packages/setuptools/build_meta.py", line 135, in _get_build_requires
      self.run_setup()
    File "/tmp/pip-build-env-eaq7p8sq/overlay/lib/python3.7/site-packages/setuptools/build_meta.py", line 259, in run_setup
      self).run_setup(setup_script=setup_script)
    File "/tmp/pip-build-env-eaq7p8sq/overlay/lib/python3.7/site-packages/setuptools/build_meta.py", line 150, in run_setup
      exec(compile(code, __file__, 'exec'), locals())
    File "setup.py", line 11, in <module>
      from breathe import __version__
    File "/tmp/pip-req-build-wrcudthy/breathe/__init__.py", line 1, in <module>
      from breathe.directives.setup import setup as directive_setup
    File "/tmp/pip-req-build-wrcudthy/breathe/directives/__init__.py", line 1, in <module>
      from breathe.finder.factory import FinderFactory
    File "/tmp/pip-req-build-wrcudthy/breathe/finder/__init__.py", line 1, in <module>
      from breathe.project import ProjectInfo
    File "/tmp/pip-req-build-wrcudthy/breathe/project.py", line 3, in <module>
      from sphinx.application import Sphinx
  ModuleNotFoundError: No module named 'sphinx'
  ----------------------------------------
WARNING: Discarding file:///home/docs/checkouts/readthedocs.org/user_builds/breathe/checkouts/latest. Command errored out with exit status 1: /home/docs/checkouts/readthedocs.org/user_builds/breathe/envs/latest/bin/python /home/docs/checkouts/readthedocs.org/user_builds/breathe/envs/latest/lib/python3.7/site-packages/pip/_vendor/pep517/in_process/_in_process.py get_requires_for_build_wheel /tmp/tmpo1quees5 Check the logs for full command output.
ERROR: Command errored out with exit status 1: /home/docs/checkouts/readthedocs.org/user_builds/breathe/envs/latest/bin/python /home/docs/checkouts/readthedocs.org/user_builds/breathe/envs/latest/lib/python3.7/site-packages/pip/_vendor/pep517/in_process/_in_process.py get_requires_for_build_wheel /tmp/tmpo1quees5 Check the logs for full command output. 

Which I can reproduce locally on my set up with a standard virtual env.

We move the top level imports into the 'setup' function in order to
allow setup.py to access the __version__ variable in the module without
having to have sphinx available.

I'm not sure if this is for the best but the ReadTheDocs build is
failing and it seems potentially related to this though I've no idea why
it is suddenly happening as we've not changed anything around this to my
understanding.
@jakobandersen
Copy link
Collaborator

I don't know much about the RTD setup, but code-wise the change looks fine to me.

@2bndy5
Copy link
Contributor

2bndy5 commented Nov 14, 2021

command: /home/docs/checkouts/readthedocs.org/user_builds/breathe/envs/latest/bin/python /home/docs/checkouts/readthedocs.org/user_builds/breathe/envs/latest/lib/python3.7/site-packages/pip/_vendor/pep517/in_process/_in_process.py get_requires_for_build_wheel /tmp/tmpo1quees5

The key phrase here is pep517 which seems to be big step towards deprecating the distutils module. As stated in PEP 517, using a pyproject.toml is now the recommended way (setup.py is the "legacy" way). I'll admit, I've been lazy in updating my python pkgs to use project.toml specs. Many people keep flocking to the python-poetry tool when faced with these ever evolving standards...

The line that throws this error is clearly

from sphinx.application import Sphinx

which is only used for typing in the __init__.py module. It is likely that the sphinx pkg is also imported in one of the other imports declared in the __init__.py module, namely breathe.renderer.sphinxrenderer.

TL;DR A Better Solution

Simply specify sphinx as a build-system requirement in the pyproject.toml:

[build-system]
requires = ["Sphinx>=3.0,<5", "docutils>=0.12"]

I tested this locally on master (without changes from this branch) and I no longer run into the error stated in the OP. And I was running into that error before.

It wouldn't hurt to refresh yourself with what the docs for pip say (they've been good at keeping that up to date with references to PEPs).


The changes on this branch won't do anything different because python's import system handles the entire module specified (obviously) - despite if the actual data being used/imported is at the top or at the bottom of the script.

@michaeljones
Copy link
Collaborator Author

Thanks for sharing your knowledge @2bndy5. I'm quite out of touch with the python ecosystem though I do reach for Poetry when I do interact with it. I just didn't know where it was considered a preferred standard. (I guess Poetry might not be but the pyproject.toml is.) I'll look into moving over.

@2bndy5
Copy link
Contributor

2bndy5 commented Nov 14, 2021

The only reason I avoid using poetry: It adds another requirement for the pkg devs. But after reviewing PEP 517 & 518, it seems easier to use a dedicated packaging system while the python standards solidify (which looks like it will take years or decades).

@michaeljones
Copy link
Collaborator Author

Thanks for providing the better approach @2bndy5 (#766)

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 this pull request may close these issues.

3 participants