Introduce `LocatedHeaderDir` to report the header discovery method by brandon-b-miller · Pull Request #1536 · NVIDIA/cuda-python

brandon-b-miller · 2026-01-27T16:20:50Z

This PR implements LocatedHeaderDir to report the discovery method of the cuda headers. Similarly to LoadedDL, we capture this info at the time the library is found and surface it through a metadata object.

After some tinkering I felt the best way of approaching this was to expand the existing API with a kwarg rather than breaking the API or introducing a parallel API that returns the struct rather than a string, which would have to maintain a separate implementation. But I'm open to other ways of doing things if they seem better.

cc @rwgk

copy-pr-bot · 2026-01-27T16:20:54Z

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.

rwgk · 2026-01-27T23:16:29Z

cuda_pathfinder/cuda/pathfinder/_headers/find_nvidia_headers.py


 @functools.cache
-def find_nvidia_header_directory(libname: str) -> str | None:
+def find_nvidia_header_directory(libname: str, include_info: bool = False) -> FoundHeader | str | None:


Cursor is telling me this is an anti-pattern, with a list of reasons, including "return type ambiguity makes static analysis harder".

I think a new function name will be best, e.g. this could become

def find_nvidia_header_dir(libname: str) -> FoundHeader | None

with something like this (untested):

def find_nvidia_header_directory(libname: str) -> str | None: found = find_nvidia_header_dir(libname) return found.abs_path if found else None

Another idea would be locate_nvidia_header_directory as the name for the new function. I almost like that better: currently we only have this one find_... function in the public API, and we don't have a locate_... function, so we could make locate the new naming convention and soft-deprecate (documentation only) the find function.

rwgk · 2026-01-27T23:16:37Z

cuda_pathfinder/cuda/pathfinder/_headers/find_nvidia_headers.py



+@dataclass
+class FoundHeader:


I think having Dir in the name is important, to clearly distinguish between a directory and file.

FoundHeaderDir or LocatedHeaderDir (see other comments)

rwgk · 2026-01-27T23:16:40Z

cuda_pathfinder/cuda/pathfinder/_headers/find_nvidia_headers.py

+    abs_path: str | None
+    found_via: str
+
+    def _normalize_path(self) -> FoundHeader:


You can use __post_init__ to do the normalization automatically when the dataclass is instantiated. This is the standard pattern for dataclasses when you need to transform fields after initialization:

@dataclass class FoundHeaderDir: abs_path: str | None found_via: str def __post_init__(self): self.abs_path = _abs_norm(self.abs_path)

With this approach, you can remove the _normalize_path() method entirely, and also remove all the ._normalize_path() calls since normalization happens automatically at construction time.

rwgk · 2026-01-27T23:16:49Z

cuda_pathfinder/cuda/pathfinder/_headers/find_nvidia_headers.py

    for cdir in candidate_dirs:
        for hdr_dir in sorted(glob.glob(cdir), reverse=True):
            if _joined_isfile(hdr_dir, h_basename):
                return _abs_norm(hdr_dir)


WDYT about

return FoundHeaderDir(abs_path=hdr_dir, found_via="supported_install_dir")

?

rwgk · 2026-01-27T23:16:54Z

cuda_pathfinder/tests/test_find_nvidia_headers.py

        assert os.path.isfile(os.path.join(hdr_dir, h_filename))
    if STRICTNESS == "all_must_work":
        assert hdr_dir is not None
+


I recommend changing the existing tests to call the richer new function, and then add only one trivial/small extra test at the bottom to exercise the old API.

I'm a little hesitant to mutate the existing tests rather than adding more - one goal of this PR is to make sure the old API remains unperturbed. What do you think of circling back to cleaning out old tests if/when we deprecate the old API?

I'd rather not maintain two parallel test implementations, that's how inconsistencies and confusion creep in over time.

The changes are trivial and super easy to review: change the function name in the calls sites and add .abs_path checks where needed. I think the insertions for assertions on .found_via will also be very obvious. One small test for the existing API wrapper will be sufficient to fully cover backward compatibility.

rwgk

This looks great to me.

rwgk · 2026-01-30T16:58:23Z

cuda_pathfinder/tests/test_find_nvidia_headers.py

+    info_summary_append(f"{None if not found_header_dir else found_header_dir.abs_path=!r}")
+    if found_header_dir:
+        # old api
+        hdr_dir = find_nvidia_header_directory(libname)


Under the if found_header_dir condition this doesn't get full test coverage.

Maybe (untested):

found_header_dir = locate_nvidia_header_directory(libname) hdr_dir = find_nvidia_header_directory(libname) # old api assert hdr_dir == None if not found_header_dir else found_header_dir.abs_path info_summary_append(f"{hdr_dir=!r}")

Then simply use hdr_dir everywhere below.

This is the only change I'd definitely make. Optional, only if you like it:

FoundHeaderDir → LocatedHeaderDir

I'd probably change most everything from "found" to "located" for full internal consistency, including _locate_under_site_packages etc.

rwgk

One more tiny thing.

Could you please trigger running the tests, after looking into my suggestion?

rwgk · 2026-01-30T19:56:12Z

cuda_pathfinder/tests/test_find_nvidia_headers.py

    info_summary_append(f"{hdr_dir=!r}")
    if hdr_dir:
+        assert isinstance(located_hdr_dir, LocatedHeaderDir)
+        assert located_hdr_dir.found_via in ("site-packages", "conda", "CUDA_HOME")


Possibly we'll get "supported_install_dir" here.

What do you think about adding a helper function (untested):

def _located_hdr_dir_asserts(located_hdr_dir): assert isinstance(located_hdr_dir, LocatedHeaderDir) assert located_hdr_dir.found_via in ("site-packages", "conda", "CUDA_HOME", "supported_install_dir")

Then call the helper from here, and right after the current line 102 below in test_locate_ctk_headers?

brandon-b-miller · 2026-01-30T20:35:32Z

/ok to test

github-actions · 2026-01-30T23:35:43Z

Doc Preview CI
Preview removed because the pull request was closed or merged.

brandon-b-miller added 3 commits January 27, 2026 08:02

initial

2e9fb5b

resolve a few tests

806c907

precommit

1f16518

rwgk reviewed Jan 27, 2026

View reviewed changes

brandon-b-miller added 2 commits January 28, 2026 07:33

address reviews

aa8e322

test updates

d5e1abe

brandon-b-miller requested a review from rwgk January 30, 2026 15:11

rwgk reviewed Jan 30, 2026

View reviewed changes

address reviews

fde70b1

brandon-b-miller requested a review from rwgk January 30, 2026 19:20

rwgk approved these changes Jan 30, 2026

View reviewed changes

address reviews

c901883

This comment has been minimized.

Sign in to view

rwgk changed the title ~~Introduce FoundHeader to report the header discovery method~~ Introduce LocatedHeaderDir to report the header discovery method Jan 30, 2026

rwgk enabled auto-merge (squash) January 30, 2026 21:41

rwgk merged commit 2cb0d68 into NVIDIA:main Jan 30, 2026
80 checks passed

Conversation

brandon-b-miller commented Jan 27, 2026 • edited by rwgk Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

copy-pr-bot bot commented Jan 27, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rwgk left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rwgk left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

brandon-b-miller commented Jan 30, 2026

Uh oh!

This comment has been minimized.

Uh oh!

github-actions bot commented Jan 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

brandon-b-miller commented Jan 27, 2026 •

edited by rwgk

Loading