Skip to content

Use pathlib in cuda.bindings build hooks and tests (part 6 of #2410) - #2501

Open
LeSingh1 wants to merge 2 commits into
NVIDIA:mainfrom
LeSingh1:pathlib/cuda-bindings-hooks
Open

Use pathlib in cuda.bindings build hooks and tests (part 6 of #2410)#2501
LeSingh1 wants to merge 2 commits into
NVIDIA:mainfrom
LeSingh1:pathlib/cuda-bindings-hooks

Conversation

@LeSingh1

@LeSingh1 LeSingh1 commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Part 6 of #2410, and the last of the series.

Replaces os.path/os.sep with pathlib.Path in cuda_bindings/build_hooks.py and in the test modules that build paths to test data, the examples directory, and a reproducer's cwd.

Values handed to setuptools' Extension (include_dirs, library_dirs) and to the cuFile string parameters stay str; only the path construction moves to pathlib.

As in part 5, glob.glob is left alone and only the pattern construction moved, and I diffed the old and new extension-discovery logic against the real source tree on Linux — identical across all 39 cuda.bindings module names and basenames, and identical include/library dir lists.

One cosmetic note: the test_examples.py parametrize IDs change from .../tests/../examples/foo.py to .../examples/foo.py. Same file set — I grepped ci/, .github/, scripts/ and toolshed/ and nothing selects these tests by ID. Happy to preserve the literal .. if you would rather keep the IDs stable.

These files all need a GPU or a built cuda.bindings, so the changes are verified by inspection plus ruff check, ruff format --check and py_compile rather than by a test run; cuda_core/tests/test_build_hooks.py still passes 14/14 on Linux CI.

NOTE: developed with the assistance of an AI coding agent. I reviewed and verified the change before submitting.

@copy-pr-bot

copy-pr-bot Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@github-actions github-actions Bot added the cuda.bindings Everything related to the cuda.bindings module label Aug 4, 2026
Comment thread cuda_bindings/tests/test_examples.py Outdated
examples_path = os.path.join(os.path.dirname(__file__), "..", "examples")
examples_files = glob.glob(os.path.join(examples_path, "**/*.py"), recursive=True)
examples_path = Path(__file__).parents[1] / "examples"
examples_files = glob.glob(str(examples_path / "**" / "*.py"), recursive=True)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why no Path.glob here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Switched to examples_path.glob("**/*.py") in 29320f2 (pushed before I saw the review). Verified it collects the same 14 files as the old glob.glob(..., recursive=True) call, and the ids=str parametrize renders the same test IDs.

Comment thread cuda_bindings/build_hooks.py Outdated
path = Path("cuda", "bindings", "_internal")
if sys.platform == "linux":
src_files = glob.glob(os.path.join(path, "*_linux.pyx"))
src_files = glob.glob(str(path / "*_linux.pyx"))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ditto

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Both are path.glob("*_linux.pyx") / path.glob("*_windows.pyx") as of 29320f2. _rename_architecture_specific_files now works in Path throughout, so dst is built with with_name() instead of string replacement on the full path.

Comment thread cuda_bindings/build_hooks.py Outdated
src_files = glob.glob(str(path / "*_linux.pyx"))
elif sys.platform == "win32":
src_files = glob.glob(os.path.join(path, "*_windows.pyx"))
src_files = glob.glob(str(path / "*_windows.pyx"))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ditto

exts = []
for pyx in files:
mod_name = pyx.replace(".pyx", "").replace(os.sep, ".").replace("/", ".")
mod_name = ".".join(Path(pyx).with_suffix("").parts)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍

@LeSingh1
LeSingh1 force-pushed the pathlib/cuda-bindings-hooks branch from 93717cf to 5d7816e Compare August 7, 2026 23:19
@LeSingh1

LeSingh1 commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

Switched both spots to Path.glob.

_rename_architecture_specific_files now yields Path, so three consumers moved with it: src.replace("_linux", "") became src.with_name(src.name.replace(...))Path.replace is os.replace, so leaving that would have been a silent footgun that renames files on disk — and f.endswith(".pyx") / Path(f).name became f.suffix / f.name.

I left _prep_extensions' glob.glob and the cuda/bindings/*.pyx listing alone. Those take a glob pattern rather than a directory, and their output goes straight into setuptools.Extension(sources=...) and cythonize, which do raw string operations — Cython's extended_iglob in particular.

Two things I checked rather than assumed:

Path.glob matches dotfiles and hidden directories where glob.glob does not. Both example trees produce identical sets today (14 files here, 20 in cuda_core), and I diffed the _rename_architecture_specific_files rewrite against a synthetic _internal/ to confirm byte-identical dst_files and resulting tree.

With bare Path values pytest degrades the parametrize IDs to example0..13, so I added ids=str and confirmed via --collect-only that IDs and collection order are byte-identical to the old string version.

Not run locally: the cuda_bindings suite and a real wheel build need CUDA and a toolkit. py_compile, ruff and ruff format are clean; CI is the gate.

Part of NVIDIA#2410. Replaces os.path/os.sep with pathlib.Path in
cuda_bindings/build_hooks.py and in the test modules that build paths
to the test data, the examples directory and the reproducer cwd.

Directory listings use Path.glob instead of glob.glob over a stringified
pattern, so _rename_architecture_specific_files() now returns Path
objects and its consumers use .name / .suffix.

Values handed to setuptools' Extension and to the cuFile string
parameters stay str; only the path construction moves to pathlib.
_prep_extensions() keeps glob.glob because its input is a glob pattern
rather than a directory.
@LeSingh1
LeSingh1 force-pushed the pathlib/cuda-bindings-hooks branch from 5d7816e to 29320f2 Compare August 8, 2026 22:53
@LeSingh1

LeSingh1 commented Aug 8, 2026

Copy link
Copy Markdown
Contributor Author

All three Path.glob comments addressed (replies inline).

One glob.glob remains, in _prep_extensions. It is not a directory search: sources[0] is a pattern that may be rooted anywhere ("cuda/bindings/utils/*.pyx") or a concrete file, so there is no Path.glob form for it — Path.glob needs the directory split out from the pattern. I trimmed the comment there to say just that. The glob.glob("cuda/bindings/*.pyx") call further down is untouched by this PR, so I left it rather than widen the diff; happy to convert both in a follow-up if you would like the module free of glob.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cuda.bindings Everything related to the cuda.bindings module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants