Skip to content

Conversation

@matthewfeickert
Copy link
Member

@matthewfeickert matthewfeickert commented Nov 20, 2025

  • Use pathlib.Path over os.path in all instances of the codebase.

* Use pathlib.Path over os.path in all instances of the codebase.
@codecov
Copy link

codecov bot commented Nov 20, 2025

Codecov Report

❌ Patch coverage is 75.00000% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.87%. Comparing base (cb5dcf2) to head (092ad2b).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/skhep_testdata/remote_files.py 75.00% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #196      +/-   ##
==========================================
- Coverage   72.38%   71.87%   -0.52%     
==========================================
  Files           3        3              
  Lines         134      128       -6     
==========================================
- Hits           97       92       -5     
+ Misses         37       36       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot finished reviewing on behalf of matthewfeickert November 20, 2025 07:21
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the codebase to use pathlib.Path instead of os.path for all file path operations, improving code readability and modernizing the code to use Python 3's pathlib module.

Key Changes:

  • Replaced os.path.join(), os.path.dirname(), os.path.exists(), os.path.isfile(), and os.makedirs() with pathlib equivalents
  • Updated type hints to accept str | Path where appropriate
  • Removed unused errno and os imports where fully replaced

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
tests/test_remote_files.py Replaced os.path.join with Path / operator for creating test file paths
tests/test_local_files.py Converted directory construction and file existence checks to use Path methods
src/skhep_testdata/remote_files.py Comprehensive refactoring of file operations to pathlib, including directory creation, file checks, and path manipulation; updated type hints to support Path objects
setup.py Replaced os.path.join with Path / operator for file path construction

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Collaborator

@ariostas ariostas 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 to me!

@matthewfeickert
Copy link
Member Author

Thanks @ariostas. I'm going to merge this then with one review, but others are welcome and if there are issues found we can do a follow up PR.

@matthewfeickert matthewfeickert merged commit d60b25d into scikit-hep:main Nov 20, 2025
13 checks passed
@matthewfeickert matthewfeickert deleted the feat/use-pathlib branch November 20, 2025 16:30
@eduardo-rodrigues
Copy link
Member

Thanks @ariostas. I'm going to merge this then with one review, but others are welcome and if there are issues found we can do a follow up PR.

This is nicer, thank you @matthewfeickert.

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

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants