Skip to content

Conversation

jarrodmillman
Copy link
Member

@jarrodmillman jarrodmillman commented Jun 30, 2023

This uses pyproject.toml and removes setup.py, .mypi.ini, and ruff.toml. I replicated the previous behavior where setup.py was grabbing optional dependencies from the requirements files using the same system we used in scikit-image (scikit-image/scikit-image#6726 and scikit-image/scikit-image#6763) and numpydoc (numpy/numpydoc#474).

pyproject.toml is autogenerated from the requirements and tools/pyproject.toml.in via python tools/generate_pyproject.toml.py. The pre-commit monitors everything to make sure things don't get out-of-sync.

@jarrodmillman jarrodmillman added this to the 3.2 milestone Jun 30, 2023
@jarrodmillman jarrodmillman marked this pull request as draft June 30, 2023 22:40
@jarrodmillman jarrodmillman force-pushed the pyproject.toml branch 4 times, most recently from fbe591a to 9facb10 Compare July 3, 2023 06:08
@jarrodmillman jarrodmillman marked this pull request as ready for review July 3, 2023 14:12
@jarrodmillman jarrodmillman requested a review from dschult July 3, 2023 14:12
Copy link
Member

@dschult dschult left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice to have this working! :}

Can you explain why we don't need wheel or setuptools anymore? What is doing the work that replaces them?

Picky nits:

  • some places use single quotes ', others use double ", some use both -> pyproject.toml.in lines ~50 and ~100. Does it matter? That is, is toml like python in its treatment of " vs '?
  • I found a skimage -> networkx conversion.

@jarrodmillman
Copy link
Member Author

jarrodmillman commented Jul 3, 2023

Can you explain why we don't need wheel or setuptools anymore? What is doing the work that replaces them?

In pyproject.toml we have

[build-system]
build-backend = 'setuptools.build_meta'
requires = ['setuptools>=61.2']

Basically, pip 10+ will create a new virtual environment, install setuptools>=61.2, and then build a wheel. setuptools requires wheel, so it will be brought in as well.

If we use --no-build-isolation, we would need to handle the virtual environment and installs ourselves.

There is more detail here https://learn.scientific-python.org/development/guides/packaging_classic/#pep-517518-support-high-priority.

@jarrodmillman
Copy link
Member Author

I think the above means we can simplify things in our CI more (perhaps). I haven't had time to look into it, but was planning to talk to Henry about it soon.

Copy link
Member

@dschult dschult left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Thanks! i approve this PR

Copy link
Contributor

@rossbar rossbar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @jarrodmillman for making this transition. I'm not an expert on PEP517/518 but from my limited understanding I think this makes sense!

@jarrodmillman jarrodmillman merged commit e3f9fd1 into networkx:main Jul 6, 2023
@jarrodmillman jarrodmillman deleted the pyproject.toml branch July 6, 2023 15:16
Alex-Markham pushed a commit to Alex-Markham/networkx that referenced this pull request Oct 13, 2023
* Use pyproject.toml

* Update pyproject.toml with requirements

* Update tools/generate_pyproject.toml.py

Co-authored-by: Dan Schult <[email protected]>

---------

Co-authored-by: Dan Schult <[email protected]>
dschult added a commit to BrunoBaldissera/networkx that referenced this pull request Oct 23, 2023
* Use pyproject.toml

* Update pyproject.toml with requirements

* Update tools/generate_pyproject.toml.py

Co-authored-by: Dan Schult <[email protected]>

---------

Co-authored-by: Dan Schult <[email protected]>
cvanelteren pushed a commit to cvanelteren/networkx that referenced this pull request Apr 22, 2024
* Use pyproject.toml

* Update pyproject.toml with requirements

* Update tools/generate_pyproject.toml.py

Co-authored-by: Dan Schult <[email protected]>

---------

Co-authored-by: Dan Schult <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants