Skip to content

BLD: Add pyproject.toml #28374

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 19 commits into from
Sep 13, 2019
Merged

Conversation

TomAugspurger
Copy link
Contributor

@TomAugspurger TomAugspurger commented Sep 10, 2019

  • Adds pyproject.toml to support this.

  • Bumps minimum Cython version to the version supporting Python 3.8.

Closes #20775
xref #27435 (check if closes)
xref #25227, #20718, #16745

* Adds pyproject.toml to support this.

* Bumps minimum Cython version to the version supporting Python 3.8.

Closes pandas-dev#28341
@TomAugspurger TomAugspurger added this to the 0.25.2 milestone Sep 10, 2019
@jbrockmendel
Copy link
Member

I take it there's no way to get the pyroject.toml stuff in setup.cfg?

@jbrockmendel
Copy link
Member

should/could we test for the desired behavior? or rare enough to check manually?

@TomAugspurger
Copy link
Contributor Author

TomAugspurger commented Sep 10, 2019 via email

@TomAugspurger
Copy link
Contributor Author

TomAugspurger commented Sep 10, 2019

I'm having trouble with editable installs. I need to pass the --no-build-isolation in order to use the extensions built with python setup.py build_ext -i.

@TomAugspurger
Copy link
Contributor Author

So from what I can tell, pip install -e . isn't going to play nicely with a previous build_ext --inplace (https://discuss.python.org/t/pip-19-1-and-installing-in-editable-mode-with-pyproject-toml/1553/68). So I guess the recommendation is to use python setup.py develop rather than python -m pip install -e .

Has anyone else followed this setup tools / pip issue? Does that sound right?

@gfyoung gfyoung added the Build Library building on various platforms label Sep 11, 2019
@jorisvandenbossche
Copy link
Member

The editable install is a known issue (for me that is a reason to not yet use pyproject.toml on other projects, but for pandas the benefits are probably bigger than the problem with pip install -e ..
Pip install will do build isolation once there is a pyproject.toml, which is something you exactly don't want (IMO) when doing an editable install.
So as you say either passing a --no-build-isolation, or either python setup.py develop.

Can we do adding the pyproject.toml separately from the stop shipping cythonized files? Eg a separate PR as precursor for this (given all the previous problems I prefer to do this very explicitly and ) Or at least not hide it in a commit / PR that seems to be about something else.

@jorisvandenbossche
Copy link
Member

So as you say either passing a --no-build-isolation, or either python setup.py develop.

From a comment that I made earlier when running into this (https://issues.apache.org/jira/browse/ARROW-5210), it seems that I needed to do pip install -e . --no-use-pep517 --no-build-isolation. But I don't remember from the top of my head why both flags were needed.

@TomAugspurger
Copy link
Contributor Author

Can we do adding the pyproject.toml separately from the stop shipping cythonized files? Eg a separate PR as precursor for this (given all the previous problems I prefer to do this very explicitly and ) Or at least not hide it in a commit / PR that seems to be about something else.

Sure. I think Pyproject.toml should be first, and needs to be back ported. I'll do that here, since it's dominated the discussion.

@TomAugspurger TomAugspurger changed the title BLD: Stop shipping cythonized files in sdist BLD: Add pyproject.toml Sep 11, 2019
@TomAugspurger
Copy link
Contributor Author

Windows people, is it possible to do a glob that expands to a file in a .bat file? E.g. I have

bash-5.0$ ls wheelhouse
pandas-0.25.0+323.g55294ca9a.dirty-cp37-cp37m-macosx_10_14_x86_64.whl
bash-5.0$ pip install wheelhouse/*.whl

That puts the single whl file in the pip install command.

@TomAugspurger
Copy link
Contributor Author

TomAugspurger commented Sep 11, 2019

Just the SSL certificate doc failure.

Edit: though this should likely wait until #28391 is in.

@TomAugspurger TomAugspurger modified the milestones: 0.25.2, 1.0 Sep 11, 2019
@jorisvandenbossche
Copy link
Member

@TomAugspurger it might be that the --no-use-pep517 is not needed, as they reverted some things in a pip bug fix release: https://pip.pypa.io/en/stable/news/#id50

@TomAugspurger
Copy link
Contributor Author

Huh, I'll try again, but I recall needing it and I'm using pip 19.2.3 (that change was in 19.1.1)

@jreback
Copy link
Contributor

jreback commented Sep 13, 2019

lgtm. merge when ready.

@TomAugspurger
Copy link
Contributor Author

it might be that the --no-use-pep517 is not needed

Oh, I understand now. The --no-use-pep517 is indeed not necessary. The --no-build-isolation is necessary to reuse previously built extensions.

@TomAugspurger
Copy link
Contributor Author

All green.

As a reminder, I'll followup with a PR to clean up our sdist (including removing the cythonized files).

@TomAugspurger
Copy link
Contributor Author

Oh, and just to make sure everyone is aware, @pandas-dev/pandas-core the proper way to rebuild extensions is now

python setup.py build_ext -i  # -j 8 for parrallel
python -m pip install -e . --no-build-isolation

if you leave off the --no-build-isolation you'll be rebuilding from scratch each time.

@TomAugspurger TomAugspurger merged commit 9dc6de3 into pandas-dev:master Sep 13, 2019
@TomAugspurger TomAugspurger deleted the sdist-cython branch September 13, 2019 16:11
@WillAyd
Copy link
Member

WillAyd commented Sep 13, 2019

@TomAugspurger just to be clear we have to run the pip install -e locally now when rebuilding instead of just python setup.py build_ext --inplace?

@jbrockmendel
Copy link
Member

I second Will's question. My usual workflow involves lots of git worktree and very little virtualenv/condaenv

@TomAugspurger
Copy link
Contributor Author

If you weren't previously using a local editable install then there's no change to your workflow.

@WillAyd
Copy link
Member

WillAyd commented Sep 13, 2019

Sounds good. We should probably update the contributing guide for that - I'll open an issue I think community will pick up

@WillAyd
Copy link
Member

WillAyd commented Sep 13, 2019

Nvm...missed already here - thanks for that

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Build Library building on various platforms
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add pyproject.toml
8 participants