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

Fix ptx file discovery in editable installs #14767

Merged

Conversation

vyasr
Copy link
Contributor

@vyasr vyasr commented Jan 17, 2024

Description

The behavior of editable installs changed when we transitioned to scikit-build-core, and it affects where the ptx files created during the build can be discovered. Editable installs no longer place built files directly alongside source files. Instead, Python's import machinery is leveraged to add built files to the search path. Since ptx files are not Python files, the loader logic isn't relevant, but we now need to ensure that we always search for the ptx files alongside built artifacts (namely Cython compiled modules) rather than Python source files. I'm guessing that nobody has encountered this yet due to preexisting build artifacts in their source directories.

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@vyasr vyasr added bug Something isn't working non-breaking Non-breaking change labels Jan 17, 2024
@vyasr vyasr self-assigned this Jan 17, 2024
@github-actions github-actions bot added Python Affects Python cuDF API. CMake CMake build issue labels Jan 17, 2024
@vyasr vyasr marked this pull request as ready for review January 17, 2024 15:35
@vyasr vyasr requested a review from a team as a code owner January 17, 2024 15:35
Copy link
Contributor

@bdice bdice left a comment

Choose a reason for hiding this comment

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

Looks fine to me. Be aware of potential conflicts with #13690. cc: @brandon-b-miller

@vyasr
Copy link
Contributor Author

vyasr commented Jan 17, 2024

Yeah I was reviewing that PR as well. I'll help Brandon deal with it as soon as this is merged.

@vyasr
Copy link
Contributor Author

vyasr commented Jan 17, 2024

/merge

@rapids-bot rapids-bot bot merged commit 56a7b95 into rapidsai:branch-24.02 Jan 17, 2024
67 checks passed
@vyasr vyasr deleted the fix/editable_install_ptx_discovery branch January 17, 2024 16:12
@mroeschke
Copy link
Contributor

Thanks! I think I encountered this before break #14653 (comment)

@vyasr
Copy link
Contributor Author

vyasr commented Jan 18, 2024

Ah yes that looks like it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working CMake CMake build issue non-breaking Non-breaking change Python Affects Python cuDF API.
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

3 participants