Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TST/CLN: Replace all tmpdir / os.path operations in tests with pathlib.Path #411

Merged
merged 2 commits into from
May 4, 2024

Conversation

brendan-ward
Copy link
Member

Resolves #400

This removes all instances of the legacy pytest tmpdir fixture, and uses the tmp_path (a pathlib.Path object) fixture instead.

Modernized the various path operations in the tests to use methods on the pathlib.Path object.

Removes unused win32.py test file; we have actual tests against windows and this is no longer used.

Note: I updated the benchmarks to use tmp_path and verified that most of them still run (some fail with missing test files), but we're not actively maintaining these.

@brendan-ward brendan-ward added this to the 0.8.0 milestone May 4, 2024
Copy link
Member

@theroggy theroggy left a comment

Choose a reason for hiding this comment

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

Nice cleanup!

pyogrio/tests/test_geopandas_io.py Outdated Show resolved Hide resolved
Copy link
Member

@jorisvandenbossche jorisvandenbossche left a comment

Choose a reason for hiding this comment

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

Looks good!

There are a few cases where we still pass str(filename) to some read function and where the str(..) can be removed now (after your previous PR to uniformize the path handling)

@brendan-ward brendan-ward merged commit f80bc8f into main May 4, 2024
20 checks passed
@brendan-ward brendan-ward deleted the issue400 branch May 4, 2024 14:07
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.

TST: replace pytest.tmpdir fixture with pytest.tmp_path
3 participants