Skip to content

BUG: support PathLike paths in DataSource methods#31906

Merged
mattip merged 5 commits into
numpy:mainfrom
VectorPeak:fix
Jul 9, 2026
Merged

BUG: support PathLike paths in DataSource methods#31906
mattip merged 5 commits into
numpy:mainfrom
VectorPeak:fix

Conversation

@VectorPeak

@VectorPeak VectorPeak commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

### PR summary

Closes #31909.

This PR makes numpy.lib._datasource accept pathlib.Path and other string path-like local paths in the same places that already accepted string paths. The affected datasource entry points now normalize path inputs with os.fspath() before the existing URL, cache, absolute-path, and file-opening logic runs.

Previously, direct pathlib.Path inputs could reach string-specific operations such as path.split(...) without conversion. This affected DataSource.abspath, DataSource.exists, DataSource.open, the module-level numpy.lib._datasource.open, and Repository path resolution. The change is intentionally scoped to datasource path normalization; URL handling, cache lookup order, compressed-file opener selection, write-mode URL rejection, and sandboxing logic are left unchanged.

The runtime tests cover Path inputs for DataSource.open, DataSource.exists, DataSource.abspath, Repository.abspath, Repository.exists, Repository.open, and the module-level open helper. The typing fixtures now use _typeshed.StrPath so pathlib.Path is accepted while bytes-path cases remain rejected.

Validation:

  • Direct regression script using Python 3.12.13 passed for the affected Path cases and preserved URL-string detection (failure_count 0).
  • python -m py_compile numpy/lib/_datasource.py passed with Python 3.12.13.
  • git diff --check passed.
  • spin build --clean was attempted locally, but this Windows environment hit MSVC fatal error C1001 internal compiler errors while compiling unrelated linalg sources, so I could not run the focused pytest target from a completed local build.

First time committer introduction

I am contributing a small bug fix to align DataSource behavior with its documented path-like support.

AI Disclosure

AI tools were used to help identify the failing path-like case, draft and validate the patch, and prepare this PR description. The proposed code changes and PR text were reviewed before submission.

VectorPeak and others added 2 commits July 8, 2026 11:37
Co-authored-by: chatgpt-codex-connector[bot] <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>
Co-authored-by: chatgpt-codex-connector[bot] <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com>

@ikrommyd ikrommyd 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.

I'd suggest opening an issue regarding what things don't work because indeed there was pathlib.Path added in the docstrings in #23822. Then one can link the PR as closing that issue.
It looks like some cases were probably missed during the testing of #23822. if you say that things do not work while they claim to support pathlib.Path.
Since the change generalizes support from pathlib.Path to path-like objects, could you update the remaining str or pathlib.Path docstrings in _datasource.py to str or path-like for consistency?

@VectorPeak

Copy link
Copy Markdown
Contributor Author

@ikrommyd OK...I will do it

@VectorPeak

Copy link
Copy Markdown
Contributor Author

Thanks @ikrommyd. I opened #31909 to describe the documented pathlib.Path cases that were not handled consistently at runtime, and the PR body now links it with Closes #31909.

I also updated the remaining _datasource.py docstrings from str or pathlib.Path to str or path-like for the affected Repository path parameters. While touching that section, I changed the listdir return type wording to list of str, since it describes returned file names rather than path-like input values.

Local validation for this update:

  • git diff --check
  • python -m py_compile numpy/lib/_datasource.py

@VectorPeak
VectorPeak requested a review from ikrommyd July 8, 2026 07:00
@ikrommyd

ikrommyd commented Jul 8, 2026

Copy link
Copy Markdown
Member

I'll do a pass over this as soon as I can. In the meantime, please fix the linting failure in the tests (you will get locally with spin lint too)

@VectorPeak

Copy link
Copy Markdown
Contributor Author

Thanks @ikrommyd! I fixed the lint failure in numpy/lib/tests/test__datasource.py by sorting the import block as Ruff requested.

Local validation now passes:

spin lint

The remaining CircleCI checks still look pending on my side and may need maintainer approval/triggering for this fork PR. Could you please rerun/trigger CI when you have a chance?

@ikrommyd ikrommyd 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.

I think it's generally alright and a reasonable improvement. I'm not sure if I would call this a bug fix or an enhancement though tbh. I'm leaning towards enhancement as it adds path-like support even though the docstrings used to incorrectly claim pathlib.Path was supported. Any suggestions from anyone?

I'd want someone else to look at the typing stub changes as I'm not good at all with typing cc @jorenham

Comment thread numpy/lib/_datasource.py
Comment on lines 165 to 172
mode : str, optional
Mode to open `path`. Mode 'r' for reading, 'w' for writing, 'a' to
append. Available modes depend on the type of object specified by
path. Default is 'r'.
destpath : str, optional
Path to the directory where the source file gets downloaded to for
use. If `destpath` is None, a temporary directory will be created.
The default path is the current directory.

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.

I think these also should be be updated to include path-like now no?

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.

Good catch, thanks. I’ll update that.

@jorenham jorenham 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.

The typing changes are good 👌

@ikrommyd ikrommyd 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.

I think this is good. if the maintainers agree, I'd probably call this PR an enhancement and not a bug.

@mattip
mattip merged commit f5f68cc into numpy:main Jul 9, 2026
89 checks passed
@mattip

mattip commented Jul 9, 2026

Copy link
Copy Markdown
Member

Thanks @VectorPeak

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BUG: DataSource methods do not consistently accept pathlib.Path inputs

4 participants