Comply with PEP 518. #7309

ghost · 2017-04-17T14:34:20Z

No description provided.

rgommers · 2017-04-17T20:24:28Z

Thanks @xoviat, good idea. Will look at this in detail soon.

rgommers · 2017-04-19T12:27:54Z

This PR looks pretty straightforward, but I don't think we should merge it before support for PEP 518 is merged in pip (any day now, pypa/pip#4144) and we can test that things work as expected.

rgommers · 2017-04-19T12:28:50Z

The PEP doesn't say anything about optional build dependencies (like Cython for building from a git repo), so I guess we'll just have to leave that out.

rgommers · 2017-06-28T10:59:52Z

I don't think we should merge it before support for PEP 518 is merged in pip

It's merged now. Not in the latest pip release yet, but this PR can be tested with pip master.

rgommers · 2017-06-29T11:07:22Z

Note that this can't be tested from a git checkout, because pip doesn't build wheels for pip install . currently: pypa/pip#4144 (comment). In the process of being fixed: pypa/pip#4562

rgommers · 2017-06-29T11:10:33Z

Testing via sdist generation did yield an issue: include pyproject.toml should be added to MANIFEST.in.

rgommers · 2017-06-29T11:35:04Z

Testing this turned up a fairly serious problem with PEP 518 (or maybe I'm misunderstanding something). The intention of the PEP is "Installation of the build system". This is further clarified like so:

... dependencies required to execute the build system (currently that means 
what dependencies are required to execute a setup.py file).

For scipy that means that numpy should be in pyproject.toml (as done in this PR). That results in scipy being built against a numpy just downloaded from PyPI (or the cached equivalent) rather than the numpy already installed on the system. Building against the installed numpy would make more sense.

So requires=['numpy'] is definitely wrong, that causes crashes or import errors if the installed numpy has a lower C API number than the latest release.

Doing requires=['numpy==1.lowest.supported'] is better, but still feels a bit odd. For one, site.cfg can be located in the installed numpy right next to system_info.py, and of course that mechanism will break. Doc at https://github.com/numpy/numpy/blob/master/numpy/distutils/system_info.py#L71, file lookup at https://github.com/numpy/numpy/blob/master/numpy/distutils/system_info.py#L273

Even if we avoid requires=['numpy'] here, I'm sure lots of people will get that wrong in other packages.....

@njsmith @takluyver opinions?

pv · 2017-06-29T11:47:26Z

Can it say `numpy>=1.lowest.supported`? Is the package installed also then even if it is installed in the system?

pv · 2017-06-29T11:58:07Z

Apparently not, it seems to call `pip install --ignore-installed ...`. Because the build itself is not be isolated from the environment in other respects, I'm not sure if this is actually sensible behavior by pip...

njsmith · 2017-06-29T12:27:36Z

My expectation is that you want to use numpy == lowest-supported. I don't understand why that doesn't work -- isn't that what you want to build against? (I see your paragraph about that being "odd", but I don't know what any of it means :-).)

pv · 2017-06-29T12:43:38Z

Why does it disallow building against numpy already installed on the system?

takluyver · 2017-06-29T12:57:54Z

Why does it disallow building against numpy already installed on the system?

In terms of the spec, it does not prevent that. But the implementation in pip will always install these requirements anew to do the build.

Might be worth bringing up on distutils-sig.

pv · 2017-06-29T15:19:13Z

So in the end it looks like this should read "numpy >= 1.8.2", and then wrangle with pip authors to fix the arguably suboptimal behavior.

ghost · 2017-06-29T18:56:50Z

Might be worth bringing up on distutils-sig.

This is not in line with the specification, but simply a pip implementation detail. If this is really a problem, then file a pip bug.

njsmith · 2017-06-29T20:55:39Z

That can't work reliably. Suppose that library A depends on numpy == 1.10.0 and on scipy, and someone does pip install A in a fresh virtualenv. There is no numpy installed, so your proposal would first scipy build against the latest numpy, and then that scipy + an old numpy would be installed, leaving the user with a broken environment.

Plus in general build-requirements can't be installed into the system environment for a number of reasons, and you'd really rather that builds use a clean environment for reproducibility and to reduce the number of cases that need testing, so using the system numpy is really not trivial. (IIRC @takluyver had to drop the feature of running builds in a real fresh environment for now due to some annoying technical problem, but I would hope pip adds it back in the future.)

Again, why do you consider pip's behavior suboptimal?

pv · 2017-06-29T21:01:27Z

someone does pip install A in a fresh virtualenv

But this is not the case that we are discussing. This is about the situation where numpy is already installed in the virtualenv.

Moreover, also in the case that you describe, won't the current behavior of pip already produce broken environment? Is there any guarantee the numpy installed in the temporary environment is older release than what finally ends up being installed in the environment --- which is necessary to guarantee ABI compatibility?

pv · 2017-06-29T21:02:27Z

The reasons why I believe pip's current behavior is suboptimal are explained in the pip issue above.
The discussion can be continued there, I think.

njsmith · 2017-06-29T21:08:42Z

But this is not the case that we are discussing. This is about the situation where numpy is already installed in the virtualenv.

Ok, but if your proposal to handle the case we're discussing will as a side effect break a case that we're not discussing, perhaps we should discuss both of them together :-)

Moreover, also in the case that you describe, won't the current behavior of pip already produce broken environment? Is there any guarantee the numpy installed in the temporary environment is older release than what finally ends up being installed in the environment?

That's why scipy should declare a build dependency on the oldest supported numpy. That's what gives you the guarantee that the resulting scipy binary will work in any environment it might be installed into.

pv · 2017-06-29T21:10:39Z

Ok, so by "my proposal" you meant the >= in pyproject.toml --- not the point that if there's no need for pip to upgrade packages, it shoudn't. Your comment is relevant only for the former.

njsmith · 2017-06-29T21:18:35Z

No, because even if pip is modified to use system packages when possible, this will only happen if the system packages satisfy the declared version constraints. So you really want pip to use system packages if possible, AND some constraint that means "I'll accept any of these versions of they're already installed, but if none of them is installed give me the oldest". That's not what >= means though.

In any case, even if you extended pip's constraint language to support that, I gave an example of where it would still produce broken installs in a comment on the pip bug...

pv · 2017-06-29T21:19:53Z

Yes, so the question is about >= vs == in the pyproject.toml. It's correct that for >= you'd need some extra markers in the generated wheel, at the minimum --- if you want things to behave more robustly than they do currently.

rgommers · 2017-06-30T07:48:25Z

(I see your paragraph about that being "odd", but I don't know what any of it means :-).)

It's distutils, my technically correct description can't be clear almost by definition:)

Let's try in a handwaving way:

imagine you want to set up an environment with numpy, scipy & co buily against OpenBLAS or MKL
this likely requires a site.cfg with the correct paths to OpenBLAS or MKL
when you install with pip, there's two reasonable places to put that site.cfg:

(1) ~/.numpy-site.cfg
(2) site.cfg in installed_numpy/distutils/site.cfg

of course if you want this to be specific to one virtualenv, then you only have option 2.
with the "put numpy in an isolated build env" as the current implementation of PEP 518 in pip, option 2 cannot work.

pv · 2017-06-30T08:28:46Z

The site.cfg stuff doesn't really work with pip. You can achieve it with `setup.py bdist_wheel` and then installing the built wheels.

rgommers · 2017-06-30T10:33:53Z

The site.cfg stuff doesn't really work with pip.

Why not? pip simply calls setup.py install --some-more-args, so it should work fine. The only thing you can't do is put it next to setup.py, because that's not possible for an sdist download. But that's option 3; options 1 and 2 should work fine.

takluyver · 2017-06-30T10:42:51Z

pip simply calls setup.py install --some-more-args

This is gradually becoming less accurate: in an increasing number of situations, pip will call setup.py bdist_wheel and then cache & install the resulting wheel. Pip 10 is likely to extend this to installing from a local directory. I'm not sure if this makes a difference to the discussion here, but I thought I'd mention it.

rgommers · 2017-06-30T10:49:05Z

I'm not sure if this makes a difference to the discussion here, but I thought I'd mention it.

You're completely right. I was sloppy with the details; it doesn't make any difference though. As long as a normal build is done which invokes numpy.distutils then it doesn't really matter if you're building a wheel or doing a direct install, and whether it's pip or python setup.py xxx. The site.cfg mechanism always works the same.

rgommers · 2017-07-13T10:55:33Z

The right version of this should be:

[build-system]
requires = [
    "wheel",
    "setuptools",
    "numpy==1.8.2; python_version<=3.4",
    "numpy==1.9.3; python_version==3.5",
    "numpy==1.12.1; python_version==3.6",
    "numpy==1.13.1; python_version>=3.7",
]

@xoviat can you update your PR?

We're going to just have to ignore the site.cfg issue, it can still be made to work with ~.numpy-site.cfg so it will only affect a very small group of users (and it's not fixable).

ghost · 2017-07-13T15:32:48Z

@rgommers Done. It's better to get this stuff hashed out sooner rather than later.

rgommers · 2017-07-14T08:26:00Z

Merged, thanks @xoviat, all!

TomAugspurger · 2017-08-24T12:57:16Z

Forgive the uninformed question, but I'm trying to add a pyproject.toml for pandas, and was basing it off Scipy's.

I've cloned scipy, and setup a clean virtual environment with

Package    Version             Location                             
---------- ------------------- -------------------------------------
Cython     0.26                
pip        10.0.0.dev0         /Users/taugspurger/sandbox/pip       
setuptools 36.2.7.post20170824 /Users/taugspurger/sandbox/setuptools
wheel      0.29.0

pip and setuptools are master as of today.

I run pip wheel . from within the scipy repository and get an exception related to the python_version<=3.4 in the pyproject.toml:

 $ pip wheel .
Processing /Users/taugspurger/sandbox/scipy
Collecting numpy>=1.8.2 (from scipy==1.0.0.dev0+84fd3a5)
  File was already downloaded /Users/taugspurger/sandbox/scipy/numpy-1.13.1-cp36-cp36m-macosx_10_6_intel.macosx_10_9_intel.macosx_10_9_x86_64.macosx_10_10_intel.macosx_10_10_x86_64.whl
Skipping numpy, due to already being wheel.
Building wheels for collected packages: scipy
Exception:
Traceback (most recent call last):
  File "/Users/taugspurger/sandbox/pip/pip/_vendor/packaging/markers.py", line 276, in __init__
    self._markers = _coerce_parse_result(MARKER.parseString(marker))
  File "/Users/taugspurger/sandbox/pip/pip/_vendor/pyparsing.py", line 1632, in parseString
    raise exc
  File "/Users/taugspurger/sandbox/pip/pip/_vendor/pyparsing.py", line 1622, in parseString
    loc, tokens = self._parse( instring, 0 )
  File "/Users/taugspurger/sandbox/pip/pip/_vendor/pyparsing.py", line 1379, in _parseNoCache
    loc,tokens = self.parseImpl( instring, preloc, doActions )
  File "/Users/taugspurger/sandbox/pip/pip/_vendor/pyparsing.py", line 3395, in parseImpl
    loc, exprtokens = e._parse( instring, loc, doActions )
  File "/Users/taugspurger/sandbox/pip/pip/_vendor/pyparsing.py", line 1379, in _parseNoCache
    loc,tokens = self.parseImpl( instring, preloc, doActions )
  File "/Users/taugspurger/sandbox/pip/pip/_vendor/pyparsing.py", line 3717, in parseImpl
    return self.expr._parse( instring, loc, doActions, callPreParse=False )
  File "/Users/taugspurger/sandbox/pip/pip/_vendor/pyparsing.py", line 1379, in _parseNoCache
    loc,tokens = self.parseImpl( instring, preloc, doActions )
  File "/Users/taugspurger/sandbox/pip/pip/_vendor/pyparsing.py", line 3378, in parseImpl
    loc, resultlist = self.exprs[0]._parse( instring, loc, doActions, callPreParse=False )
  File "/Users/taugspurger/sandbox/pip/pip/_vendor/pyparsing.py", line 1379, in _parseNoCache
    loc,tokens = self.parseImpl( instring, preloc, doActions )
  File "/Users/taugspurger/sandbox/pip/pip/_vendor/pyparsing.py", line 3545, in parseImpl
    raise maxException
  File "/Users/taugspurger/sandbox/pip/pip/_vendor/pyparsing.py", line 3530, in parseImpl
    ret = e._parse( instring, loc, doActions )
  File "/Users/taugspurger/sandbox/pip/pip/_vendor/pyparsing.py", line 1379, in _parseNoCache
    loc,tokens = self.parseImpl( instring, preloc, doActions )
  File "/Users/taugspurger/sandbox/pip/pip/_vendor/pyparsing.py", line 3717, in parseImpl
    return self.expr._parse( instring, loc, doActions, callPreParse=False )
  File "/Users/taugspurger/sandbox/pip/pip/_vendor/pyparsing.py", line 1379, in _parseNoCache
    loc,tokens = self.parseImpl( instring, preloc, doActions )
  File "/Users/taugspurger/sandbox/pip/pip/_vendor/pyparsing.py", line 3395, in parseImpl
    loc, exprtokens = e._parse( instring, loc, doActions )
  File "/Users/taugspurger/sandbox/pip/pip/_vendor/pyparsing.py", line 1379, in _parseNoCache
    loc,tokens = self.parseImpl( instring, preloc, doActions )
  File "/Users/taugspurger/sandbox/pip/pip/_vendor/pyparsing.py", line 3545, in parseImpl
    raise maxException
  File "/Users/taugspurger/sandbox/pip/pip/_vendor/pyparsing.py", line 3530, in parseImpl
    ret = e._parse( instring, loc, doActions )
  File "/Users/taugspurger/sandbox/pip/pip/_vendor/pyparsing.py", line 1379, in _parseNoCache
    loc,tokens = self.parseImpl( instring, preloc, doActions )
  File "/Users/taugspurger/sandbox/pip/pip/_vendor/pyparsing.py", line 3545, in parseImpl
    raise maxException
  File "/Users/taugspurger/sandbox/pip/pip/_vendor/pyparsing.py", line 3530, in parseImpl
    ret = e._parse( instring, loc, doActions )
  File "/Users/taugspurger/sandbox/pip/pip/_vendor/pyparsing.py", line 1383, in _parseNoCache
    loc,tokens = self.parseImpl( instring, preloc, doActions )
  File "/Users/taugspurger/sandbox/pip/pip/_vendor/pyparsing.py", line 2413, in parseImpl
    raise ParseException(instring, loc, self.errmsg, self)
pip._vendor.pyparsing.ParseException: Expected {Group:({{{"implementation_version" | "platform_python_implementation" | "implementation_name" | "python_full_version" | "platform_release" | "platform_version" | "platform_machine" | "platform_system" | "python_version" | "sys_platform" | "os_name" | "os.name" | "sys.platform" | "platform.version" | "platform.machine" | "platform.python_implementation" | "python_implementation" | "extra"} | {quoted string, starting with ' ending with ' | quoted string, starting with " ending with "}} {"===" | "==" | ">=" | "<=" | "!=" | "~=" | ">" | "<" | "not in" | "in"} {{"implementation_version" | "platform_python_implementation" | "implementation_name" | "python_full_version" | "platform_release" | "platform_version" | "platform_machine" | "platform_system" | "python_version" | "sys_platform" | "os_name" | "os.name" | "sys.platform" | "platform.version" | "platform.machine" | "platform.python_implementation" | "python_implementation" | "extra"} | {quoted string, starting with ' ending with ' | quoted string, starting with " ending with "}}}) | Group:({Suppress:("(") Forward: ... Suppress:(")")})} (at char 16), (line:1, col:17)

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Users/taugspurger/sandbox/pip/pip/basecommand.py", line 214, in main
    status = self.run(options, args)
  File "/Users/taugspurger/sandbox/pip/pip/commands/wheel.py", line 188, in run
    wheels_built_successfully = wb.build(session=session)
  File "/Users/taugspurger/sandbox/pip/pip/wheel.py", line 793, in build
    python_tag=python_tag,
  File "/Users/taugspurger/sandbox/pip/pip/wheel.py", line 648, in _build_one
    self._install_build_reqs(build_reqs, prefix)
  File "/Users/taugspurger/sandbox/pip/pip/wheel.py", line 628, in _install_build_reqs
    for r in reqs]
  File "/Users/taugspurger/sandbox/pip/pip/wheel.py", line 628, in <listcomp>
    for r in reqs]
  File "/Users/taugspurger/sandbox/pip/pip/req/req_install.py", line 184, in from_line
    markers = Marker(markers)
  File "/Users/taugspurger/sandbox/pip/pip/_vendor/packaging/markers.py", line 280, in __init__
    raise InvalidMarker(err_str)
pip._vendor.packaging.markers.InvalidMarker: Invalid marker: 'python_version<=3.4', parse error at '3.4'

Does that seem like a bug in setuptools / pip, or is it a problem with scipy's pyproject.toml? (Or, more likely, am I doing something wrong?)

takluyver · 2017-08-24T13:16:29Z

I think 3.4 needs to be quoted.

ghost · 2017-08-24T13:18:47Z

FYI PEP 518 is not implemented in pip.

pv · 2017-08-24T13:20:39Z

@xoviat: pypa/pip#4144

TomAugspurger · 2017-08-24T13:21:42Z

Yeah, and I'm using pip master. Quoting 3.4 lead to a different error. I'll keep debugging this and report back (hopefully with a PR). Thanks!

ghost · 2017-08-24T13:23:54Z

No. pip does not implement 518. They tried to implement it and failed. I'm working on a fix. On Aug 24, 2017 8:21 AM, "Tom Augspurger" <notifications@github.com> wrote: Yeah, and I'm using pip master. Quoting 3.4 lead to a different error. I'll keep debugging this and report back (hopefully with a PR). — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#7309 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AOo8_Dpaf0rBitQVAuVJy1WgJIjb2Hlvks5sbXjtgaJpZM4M_Mlm> .

pv · 2017-08-24T13:29:29Z

But I guess we should add the double quotes (or single quotes) in the environment markers, as they seem to be required by PEP 508: gh-7792

ghost · 2017-08-24T13:49:13Z

Also, should we add cython?

pv · 2017-08-24T13:49:30Z

Yep.

Comply with PEP 518.

1936387

rgommers self-requested a review April 17, 2017 20:20

rgommers added Official binaries Items related to the official SciPy binaries, including wheels and vendored libs enhancement A new feature or improvement labels Apr 17, 2017

rgommers added this to the 1.0 milestone Apr 17, 2017

ghost mentioned this pull request Jun 29, 2017

PEP 518 build requirements cannot be overriden by user pypa/pip#4582

Open

PEP 518: fixup

63ba10d

rgommers merged commit d83699f into scipy:master Jul 14, 2017

ghost deleted the patch-1 branch July 14, 2017 16:13

TomAugspurger mentioned this pull request Aug 23, 2017

PKG: Added pyproject.toml for PEP 518 pandas-dev/pandas#16745

Merged

Comply with PEP 518. #7309

Comply with PEP 518. #7309

Conversation

ghost commented Apr 17, 2017

rgommers commented Apr 17, 2017

rgommers commented Apr 19, 2017

rgommers commented Apr 19, 2017

rgommers commented Jun 28, 2017

rgommers commented Jun 29, 2017

rgommers commented Jun 29, 2017

rgommers commented Jun 29, 2017

pv commented Jun 29, 2017 via email • edited by rgommers Loading

pv commented Jun 29, 2017 via email

njsmith commented Jun 29, 2017

pv commented Jun 29, 2017 via email

takluyver commented Jun 29, 2017

pv commented Jun 29, 2017 via email

ghost commented Jun 29, 2017

njsmith commented Jun 29, 2017

pv commented Jun 29, 2017 • edited Loading

pv commented Jun 29, 2017 • edited Loading

njsmith commented Jun 29, 2017

pv commented Jun 29, 2017

njsmith commented Jun 29, 2017

pv commented Jun 29, 2017 • edited Loading

rgommers commented Jun 30, 2017

pv commented Jun 30, 2017 via email

rgommers commented Jun 30, 2017

takluyver commented Jun 30, 2017

rgommers commented Jun 30, 2017

rgommers commented Jul 13, 2017

ghost commented Jul 13, 2017

rgommers commented Jul 14, 2017

TomAugspurger commented Aug 24, 2017

takluyver commented Aug 24, 2017

ghost commented Aug 24, 2017

pv commented Aug 24, 2017

TomAugspurger commented Aug 24, 2017 • edited Loading

ghost commented Aug 24, 2017 via email

pv commented Aug 24, 2017 • edited Loading

ghost commented Aug 24, 2017

pv commented Aug 24, 2017

pv commented Jun 29, 2017 via email •

edited by rgommers

Loading

pv commented Jun 29, 2017 •

edited

Loading

pv commented Jun 29, 2017 •

edited

Loading

pv commented Jun 29, 2017 •

edited

Loading

TomAugspurger commented Aug 24, 2017 •

edited

Loading

pv commented Aug 24, 2017 •

edited

Loading