Skip to content

Conversation

@mwtoews
Copy link
Contributor

@mwtoews mwtoews commented Aug 8, 2025

This fixes an issue reading PathLike objects with the shp, shx and dbf args. For example:

shapefile.Reader(dbf=Path("shapefiles") / "blockgroups.dbf")

raises:

shapefile.ShapefileException: Could not load shapefile constituent file from: /path/to/pyshp/shapefiles/blockgroups.dbf

src/shapefile.py:2447: ShapefileException

Other changes:

  • Use simpler PathLike alias instead of os.PathLike, as this is a highly-visible object.
  • Change test_shapefile.py to use mostly Path objects.
  • Change test_reader_pathlike to test_reader_path_str to test reading from a str path.
  • Use tmp_pth pytest fixture to write "corrupt_too_long" file instead of modifying files in the tests directory.

@JamesParrott
Copy link
Collaborator

JamesParrott commented Aug 8, 2025

There're 4 or 5 things going on here:
i) adjust type hints in run_benchmarks.py ✅
ii) adjust type hints in shapefile.py ✅
iii) support Paths in Reader's shp, dbf and shx kwargs ✅
iv) Make 106 separate changes to the existing tests in test_shapefile.py (to use paths instead of strings), many of which
affect Reader's first positional arg (shapefile_path) instead of the shp, dbf or shx kwargs ❌
v) No new tests have been added ❌

i) ii) and iii) are nice changes. It would be good to have more pathlib.Path tests in test_shapefile.py other
than test_reader_pathlike too.

This does not mean we should ditch a single one of the existing tests that pass in strings to Reader.

@JamesParrott JamesParrott reopened this Aug 9, 2025
@JamesParrott
Copy link
Collaborator

JamesParrott commented Aug 9, 2025

I restored the old test_shapefile.py, copied in and renamed the altered tests (from the original commit on this PR) that passed a Path to Reader as shp, dbf or shx kwargs, then rebased on master (to pick up ditching of isort).

@JamesParrott JamesParrott merged commit e797520 into GeospatialPython:master Aug 9, 2025
26 checks passed
@mwtoews mwtoews deleted the fix-pathlike branch August 9, 2025 22:23
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.

2 participants