BUG: respect pathsep for f2py include paths#31934
Conversation
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.
|
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. |
|
Sorry, my bad. |
Generally looks OK but I'll take a look over the weekend to be sure, thanks for the ping! |
HaoZeke
left a comment
There was a problem hiding this comment.
On second thought this is small enough — LGTM.
--include-pathsis 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 asC:\....PR summary
Fixes #31933.
What Problem This Solves
f2py --include-pathsis 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-pathsand the deprecated--include_pathsform on:unconditionally.That is incorrect for native Windows paths, because a drive-letter path such as
C:\inc1contains: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-pathsand deprecated--include_pathsparser branch to split onos.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
-Ihandling.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:
numpy/f2py/f2py2e.pydefinitions:--include-paths C:\inc1remains one path--include-paths C:\inc1;D:\inc2splits into two paths whenos.pathsep == ;--include_pathsfollows the same path-list behavior-I C:\inc1;D:\inc2remains one single pathgit diff --checkRemote validation:
First time committer introduction
This change was found while reviewing F2PY path handling on Windows, especially the difference between path-list parsing for
--include-pathsand 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.