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

ensure the setup.py directory is in sys.path because of PEP 517 #1273

Closed
wants to merge 1 commit into from

Conversation

ngoldbaum
Copy link

@ngoldbaum ngoldbaum commented Feb 6, 2019

Rationale

Currently cartopy is failing to build on e.g. Travis with the following traceback:

 Traceback (most recent call last):
    File "/home/travis/build/ngoldbaum/yt/venv/lib/python2.7/site-packages/pip/_vendor/pep517/_in_process.py", line 207, in <module>
      main()
    File "/home/travis/build/ngoldbaum/yt/venv/lib/python2.7/site-packages/pip/_vendor/pep517/_in_process.py", line 197, in main
      json_out['return_val'] = hook(**hook_input['kwargs'])
    File "/home/travis/build/ngoldbaum/yt/venv/lib/python2.7/site-packages/pip/_vendor/pep517/_in_process.py", line 54, in get_requires_for_build_wheel
      return hook(config_settings)
    File "/tmp/pip-build-env-dD1vz4/overlay/lib/python2.7/site-packages/setuptools/build_meta.py", line 130, in get_requires_for_build_wheel
      return self._get_build_requires(config_settings, requirements=['wheel'])
    File "/tmp/pip-build-env-dD1vz4/overlay/lib/python2.7/site-packages/setuptools/build_meta.py", line 112, in _get_build_requires
      self.run_setup()
    File "/tmp/pip-build-env-dD1vz4/overlay/lib/python2.7/site-packages/setuptools/build_meta.py", line 126, in run_setup
      exec(compile(code, __file__, 'exec'), locals())
    File "setup.py", line 36, in <module>
      import versioneer
  ImportError: No module named versioneer

The issue is that in the latest version of pip, they enabled PEP 517 and PEP 518 support, which means that the directory of setup.py is no longer included in sys.path. Since cartopy uses versioneer and we want that behavior, the fix is to explicitly add the path to the setup.py directory to sys.path.

This was suggested to me by @pganssle on IRC.

Implications

I think this is harmless for older setuptools and pip verisons, because . is already in sys.path.

@SciTools-assistant SciTools-assistant added the Blocked: CLA needed See https://scitools.org.uk. Submit the form at: https://scitools.org.uk/cla/v4/form label Feb 6, 2019
@ngoldbaum
Copy link
Author

I signed the CLA.

@SciTools-assistant SciTools-assistant removed the Blocked: CLA needed See https://scitools.org.uk. Submit the form at: https://scitools.org.uk/cla/v4/form label Feb 6, 2019
@pganssle
Copy link

pganssle commented Feb 6, 2019

One thing to note is that even though, after the next pip release, this won't be necessary, you will need to deal with this when you eventually explicitly specify setuptools.build_meta as your PEP 517 backend. As long as your build system works with PEP 517, you can also explicitly specify setuptools.build_meta as your backend in your pyproject.toml, since you won't be relying on the legacy behavior.

[build-system]
requires = [
    "setuptools >= 0.40.8",
    "wheel",
    "Cython >= 0.28",
    "numpy >= 1.10",
]
build-backend="setuptools.build_meta"

I recommend doing this in a separate PR, though. Your build is complicated enough that you should probably test the transition better than you need to test this.

Note: I have not tested this, but I think that you can also add "versioneer" to your requires and remove HERE from your setup.py's sys.path once you switch to a PEP 517 build.

@ngoldbaum
Copy link
Author

And I guess this isn't strictly necessary to build cartopy because pyproject.toml was removed in #1264. But it would be necessary if you ever re-added pyproject.toml.

@ocefpaf
Copy link
Member

ocefpaf commented Feb 11, 2020

I was looking into this recently and maybe the best course of action is to drop versioneer instead of append the current path to sys.path. At @ngoldbaum and @pganssle do you know any nice versioneer alternatives out there?

@ngoldbaum
Copy link
Author

I think setuptools_scm plays more nicely with pep517.

@ocefpaf
Copy link
Member

ocefpaf commented Feb 11, 2020

I think setuptools_scm plays more nicely with pep517.

Thanks! I'll play with it in other projects I have more control and, once I feel comfortable with it, I'll send a PR here. (Unless someone beats me to it first.)

@QuLogic
Copy link
Member

QuLogic commented Feb 12, 2020

I do like setuptools_scm, especially since we have some patched copy of versioneer, which also seems dead upstream. If it fixes things, I would be happy to start using it.

@dopplershift
Copy link
Contributor

I've switched MetPy over to setuptools_scm (if for no other reason than it's alive and under the PyPA umbrella). So far, no complaints. The only gotcha is that it's actual main purpose in life is to include everything under source control in release tarballs; for MetPy this took some adjusting of things, but in the end I think the product was better.

IIRC, the versioneer here is tweaked, so we'd have to look at either changing the versioning behavior or adjusting to something new.

@QuLogic
Copy link
Member

QuLogic commented Mar 22, 2020

I looked at setuptools_scm, and I think it requires a new-ish version of setuptools. I don't know if we can rely on that for Python 2, but as soon as 0.18 is out and we drop support for Py2, then I'll look at it again.

@QuLogic
Copy link
Member

QuLogic commented Apr 30, 2020

I've opened #1545 to switch to setuptools-scm.

@QuLogic
Copy link
Member

QuLogic commented Jul 14, 2020

So I think we don't need this any more with the above PR merged.

@QuLogic QuLogic closed this Jul 14, 2020
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.

6 participants