Skip to content

Add a pyproject.toml file #269

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

Merged
merged 2 commits into from
May 13, 2024
Merged

Conversation

vyasr
Copy link
Contributor

@vyasr vyasr commented May 13, 2024

By explicitly specifying the legacy setuptools build backend via pyproject.toml, we ensure that the current directory is visible to the setup.py invocation. Without this, in some environments I observe that pip installing the sdist or a clone of the repo fails because the current directory is not on the PYTHONPATH (for instance, using cpplint as a pre-commit hook fails for me on some Python/pip versions). Note that this is because the setup.py in this repo imports the cpplint module from the current directory, and for this reason it is necessary to specify that the build backend is setuptools.build_meta:__legacy__, not just setuptools.build_meta.

@vyasr
Copy link
Contributor Author

vyasr commented May 13, 2024

I don't fully know how to read the output, but it looks like the failing test is because the index of something in a list of errors has changed from 5 to 4:

  E       -'src/descriptor_unittest.cc:5929:  Single-parameter constructors should be marked explicit.  [runtime/explicit] [5]\n'
  E       -'src/descriptor_unittest.cc:5968:  Single-parameter constructors should be marked explicit.  [runtime/explicit] [5]\n'
  E       +'src/descriptor_unittest.cc:5929:  Single-parameter constructors should be marked explicit.  [runtime/explicit] [4]\n'
  E       +'src/descriptor_unittest.cc:5968:  Single-parameter constructors should be marked explicit.  [runtime/explicit] [4]\n'

It may be related to this changeset insofar as the output of some build step could like quite different now because pip outputs different lines (and runs some different setup) solely on the basis of pyproject.toml existing. That's just a guess, though; I'm afraid I don't really know what this test is intended for.

@aaronliu0130
Copy link
Member

aaronliu0130 commented May 13, 2024

Yes, it's unrelated. See #268. Unfortunately, our other maintainer @jayvdb is not active, and we cannot merge our own PRs.

I'll use a sockpuppet to just get that fix in.

@aaronliu0130
Copy link
Member

Merged. @vyasr Could you sync with our latest commit?

@vyasr
Copy link
Contributor Author

vyasr commented May 13, 2024

Merged the latest develop, thanks!

Copy link
Member

@aaronliu0130 aaronliu0130 left a comment

Choose a reason for hiding this comment

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

lgtm, hopefully we can get the entire setup.py converted to toml someday!

@aaronliu0130 aaronliu0130 merged commit 22c0fe2 into cpplint:develop May 13, 2024
@vyasr vyasr deleted the feat/add_pyproject branch November 27, 2024 23:26
rapids-bot bot pushed a commit to rapidsai/ucxx that referenced this pull request Dec 2, 2024
This PR updates our pre-commit to use the latest version of cpplint. This brings in [my fix](cpplint/cpplint#269) that ensures that the pre-commit hook actually works in all environments. Without that fix, depending on the version of Python and setuptools in the base environment the cpplint installation may fail.

Authors:
  - Vyas Ramasubramani (https://github.com/vyasr)
  - Peter Andreas Entschev (https://github.com/pentschev)

Approvers:
  - Peter Andreas Entschev (https://github.com/pentschev)
  - Kyle Edwards (https://github.com/KyleFromNVIDIA)

URL: #338
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.

2 participants