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

Switch to pyproject.toml package setup #306

Merged
merged 32 commits into from
May 15, 2024

Conversation

GenevieveBuckley
Copy link
Collaborator

Closes #305

pyproject.toml Outdated Show resolved Hide resolved
@m-albert m-albert mentioned this pull request Aug 2, 2023
@m-albert
Copy link
Collaborator

m-albert commented Aug 2, 2023

I've noticed that setup.py is explicitly referenced in several places: https://github.com/search?q=repo%3Adask%2Fdask-image+setup.py&type=code

Perhaps most critically in

python setup.py sdist bdist_wheel

(replacement might be python -m build? Not sure whether build would need to be added as a dependency)

and

python setup.py install

underlying the gpuci failure (replacement might be python -m pip install -e . ?).

@m-albert m-albert mentioned this pull request Aug 2, 2023
pyproject.toml Outdated Show resolved Hide resolved
@jakirkham
Copy link
Member

Yeah in terms of CI changes with setup.py. Would suggest going the pip install route for now. That is pretty close to what we were doing before

pyproject.toml Outdated Show resolved Hide resolved
Co-authored-by: jakirkham <[email protected]>
Copy link
Member

@jakirkham jakirkham left a comment

Choose a reason for hiding this comment

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

Thanks Genevieve! 🙏

Think this is coming along nicely 😄

Tried to help answer some questions and suggest some small changes on some of the trickier pieces

pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Outdated
markers = "cupy"

[tool.flake8]
exclude = "docs/conf.py,versioneer.py"
Copy link
Member

Choose a reason for hiding this comment

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

Whether to keep versioneer.py here depends on the versioning decision above ( #306 (comment) )

pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Show resolved Hide resolved
GenevieveBuckley and others added 4 commits August 4, 2023 12:32
Co-authored-by: jakirkham <[email protected]>
Co-authored-by: jakirkham <[email protected]>
Co-authored-by: jakirkham <[email protected]>
Co-authored-by: jakirkham <[email protected]>
Co-authored-by: jakirkham <[email protected]>
@m-albert
Copy link
Collaborator

The conflict is probably due to PR ( #336 ). Though think we can just delete the file again during conflict resolution

I merged main to start testing this branch

@m-albert
Copy link
Collaborator

The gpuCI fails with the following errpr:

09:53:55 python: can't open file '/workspace/setup.py': [Errno 2] No such file or directory

Could it be that "setup.py" is mentioned somewhere in the Jenkins configuration?

@jakirkham
Copy link
Member

jakirkham commented Mar 19, 2024

From CI:

python: can't open file '/workspace/setup.py': [Errno 2] No such file or directory

Looks like this code needs an update:

rapids-logger "Install dask-image"
python setup.py install

Edit: It is probably worth using git grep to find any more setup.py references and replacing them. A quick GH search reveals a few setup.py usages (notably in docs)

@m-albert
Copy link
Collaborator

I had overseen the code you referenced somehow, thanks for the clarification John 🙏

@GenevieveBuckley Would you like me to help completing this PR?

@GenevieveBuckley
Copy link
Collaborator Author

PR #370 adds python 3.12 support. This PR should also have python 3.12 support.

@GenevieveBuckley
Copy link
Collaborator Author

@GenevieveBuckley Would you like me to help completing this PR?

Sorry I didn't see your message earlier @m-albert
Of course, if you want to give it a go I'm very happy for you to do that 😄

@GenevieveBuckley
Copy link
Collaborator Author

Decision: I'm going to try replacing versioneer with setuptools-scm as part of this PR.
It seems simpler for maintenance, and looks like it will have better long term support. Fingers crossed for a smooth transition 🤞

@GenevieveBuckley
Copy link
Collaborator Author

Ok, done. There's still a few GPU CI things left to take care of, and that work is happening in separate PRs.

@GenevieveBuckley GenevieveBuckley merged commit 56e307f into dask:main May 15, 2024
17 of 18 checks passed
@jakirkham
Copy link
Member

Hooray! 🎉

Thanks Genevieve! 🙏

@m-albert
Copy link
Collaborator

@GenevieveBuckley 🚀

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.

Move from setup.py to pyproject.toml
3 participants