-
Notifications
You must be signed in to change notification settings - Fork 243
Cython .pth file support for pixi path dependencies #1562
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
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
/ok to test |
This comment has been minimized.
This comment has been minimized.
cpcloud
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I ... kind of love and hate this PR.
| # If cuda-bindings isn't available yet, we can't add the path | ||
| # This might happen in some build scenarios, but it's okay - the | ||
| # wildcard dependency will work in those cases | ||
| print("cuda-bindings not found in current environment, skipping .pth modification") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| print("cuda-bindings not found in current environment, skipping .pth modification") |
I'm not sure that's helpful here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd err on the side of printing out an error message rather than failing silently.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In that case, let's also print to stderr (via file=sys.stderr)
|
Auto-sync is disabled for ready for review pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
Co-authored-by: Phillip Cloud <417981+cpcloud@users.noreply.github.com>
Co-authored-by: Phillip Cloud <417981+cpcloud@users.noreply.github.com>
Co-authored-by: Phillip Cloud <417981+cpcloud@users.noreply.github.com>
Co-authored-by: Phillip Cloud <417981+cpcloud@users.noreply.github.com>
Co-authored-by: Phillip Cloud <417981+cpcloud@users.noreply.github.com>
Co-authored-by: Phillip Cloud <417981+cpcloud@users.noreply.github.com>
|
/ok to test |
|
Problem
cuda-coredepends oncuda-bindingsat build time and needs access to Cython.pxdheader files. Previously, we used wildcard dependencies (cuda-bindings = "*") because Cython cannot find.pxdfiles through Python's editable install meta path finders ([Cython issue #7326] cython/cython#7326)). This meant pixi couldn't track local dependencies, so rebuilds had to be managed manually.Solution
Implemented the .pth file workaround from scikit-build-core PR#516:
sys.pathso Cython can find.pxdfiles during compilation.pthfile to maintain the path for importsThis enables changing from:
cuda-bindings = "*" # No dependency tracking
To:
cuda-bindings = { path = "../cuda_bindings" } # Full dependency tracking