Skip to content

BUG: respect pathsep for f2py include paths#31934

Merged
HaoZeke merged 2 commits into
numpy:mainfrom
VectorPeak:f2py-include-pathsep
Jul 9, 2026
Merged

BUG: respect pathsep for f2py include paths#31934
HaoZeke merged 2 commits into
numpy:mainfrom
VectorPeak:f2py-include-pathsep

Conversation

@VectorPeak

@VectorPeak VectorPeak commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

--include-paths is documented as a path-list option, so it should be parsed using the current platform's path separator. However, the implementation currently splits it with a hard-coded :, which incorrectly splits Windows drive-letter paths such as C:\....

PR summary

Fixes #31933.

What Problem This Solves

f2py --include-paths is documented as a path-list option whose entries should be separated with the platform path separator (os.pathsep): : on POSIX and ; on Windows. The current parser instead splits --include-paths and the deprecated --include_paths form on : unconditionally.

That is incorrect for native Windows paths, because a drive-letter path such as C:\inc1 contains : as part of the path itself. As a result, the parser can split a single Windows include directory into invalid pieces before F2PY passes the include path list downstream.

Changes

This PR updates the --include-paths and deprecated --include_paths parser branch to split on os.pathsep. It leaves -I <path> unchanged as a single include-directory option.

 if option_string in {--include-paths, --include_paths}:
-    include_paths_set.update(values.split(':'))
+    include_paths_set.update(values.split(os.pathsep))
 else:
     include_paths_set.add(values)

The change is intentionally scoped to F2PY CLI option parsing and the corresponding help/docs text. It does not change Fortran include-file resolution, Meson/backend include propagation, or compiler -I handling.

The PR also updates the CLI help/docs wording from the POSIX-looking <path1>:<path2> form to the platform-neutral <path1><pathsep><path2> form, and adds parser-level regression coverage for Windows-style drive-letter paths and Windows-style path lists.

Evidence

Validation performed locally:

  • Parser-only check against the patched numpy/f2py/f2py2e.py definitions:
    • --include-paths C:\inc1 remains one path
    • --include-paths C:\inc1;D:\inc2 splits into two paths when os.pathsep == ;
    • deprecated --include_paths follows the same path-list behavior
    • -I C:\inc1;D:\inc2 remains one single path
  • git diff --check

Remote validation:

  • CI passed on this PR, including lint, Windows, Linux, macOS, wheel builds, MyPy, stubtest, and CircleCI docs/build checks.

First time committer introduction

This change was found while reviewing F2PY path handling on Windows, especially the difference between path-list parsing for --include-paths and single-path handling for -I.

AI Disclosure

AI assisted with identifying the issue and drafting the patch. The final code was reviewed and submitted by the human contributor.

Use os.pathsep when parsing f2py --include-paths so Windows drive-letter paths are not split on ':'. Keep -I as a single-path option and add parser-level coverage for both forms.
@ngoldbaum

Copy link
Copy Markdown
Member

This looks right to me on first-glance. @HaoZeke are you at all interested in taking a look?

@VectorPeak some feedback for your contribution style. Please keep in mind that humans are reading your issues and PR descriptions. Try to keep them as short as possible. AI models like to spew out far too much text for a reviewer to read in a reasonable amount of time.

@VectorPeak

Copy link
Copy Markdown
Contributor Author

Sorry, my bad.

@HaoZeke

HaoZeke commented Jul 9, 2026

Copy link
Copy Markdown
Member

This looks right to me on first-glance. @HaoZeke are you at all interested in taking a look?

@VectorPeak some feedback for your contribution style. Please keep in mind that humans are reading your issues and PR descriptions. Try to keep them as short as possible. AI models like to spew out far too much text for a reviewer to read in a reasonable amount of time.

Generally looks OK but I'll take a look over the weekend to be sure, thanks for the ping!

@HaoZeke HaoZeke left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

On second thought this is small enough — LGTM.

@HaoZeke
HaoZeke merged commit ae4167a into numpy:main Jul 9, 2026
87 checks passed
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.

BUG: f2py --include-paths does not respect os.pathsep on Windows

3 participants