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

Migrate as much as possible to pyproject.toml #3317

Merged
merged 15 commits into from
Mar 16, 2023

Conversation

vyasr
Copy link
Contributor

@vyasr vyasr commented Mar 7, 2023

This moves as much packaging information and linter configuration out of setup.[py|cfg] files into pyproject.toml with the aim of migrating to more modern Python builders in the future.

@vyasr vyasr added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Mar 7, 2023
@vyasr vyasr self-assigned this Mar 7, 2023
@vyasr vyasr requested review from a team as code owners March 7, 2023 19:28
@vyasr
Copy link
Contributor Author

vyasr commented Mar 7, 2023

@rlratzel @jakirkham the conda split recipe workaround being used for the service build is causing problems here. I'm fairly certain that the --no-index argument to pip is causing it to not use the index even for installing build dependencies (the stuff under build-system in pyproject.toml). I've tried adding them to the conda recipe in case that just works but I'm expecting it to still fail without further changes. Should we be building those packages with build isolation turned off? I believe that (in conjunction with the recipe changes) would fix the problem since then the package would be built in the environment created by conda-build that would contain the requirements specified in the recipe.

@vyasr
Copy link
Contributor Author

vyasr commented Mar 8, 2023

Yep build isolation did it. Worked before because there was no pyproject.toml file at all so it wasn't using a PEP 518 build at all.

@jakirkham
Copy link
Member

Yeah disabling build isolation makes sense

Normally conda-build passes this option to pip, but the recipe here runs into an edge case where these flags need to be passed manually ( conda/conda-build#3993 )

Copy link
Contributor

@rlratzel rlratzel left a comment

Choose a reason for hiding this comment

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

Thanks for doing this. I have some minor change requests.

python/cugraph-dgl/pyproject.toml Outdated Show resolved Hide resolved
python/cugraph-dgl/pyproject.toml Outdated Show resolved Hide resolved
python/cugraph-service/server/pyproject.toml Show resolved Hide resolved
python/pylibcugraph/pyproject.toml Outdated Show resolved Hide resolved
@rlratzel
Copy link
Contributor

/merge

@rapids-bot rapids-bot bot merged commit 107205c into rapidsai:branch-23.04 Mar 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Improvement / enhancement to an existing function non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants