-
Notifications
You must be signed in to change notification settings - Fork 259
ensure Python 3.14 compatibility #877
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
Conversation
Reviewer's GuideAdds Python 3.14 to CI, refines file URL parsing with deprecation warnings, standardizes test URL prefixes for Windows, updates PEP 517 backend test to use URIs, and conditionally imports Traversable based on Python version. Sequence diagram for file URL parsing and warning logic in utils.pysequenceDiagram
participant "url_to_path()"
participant "re"
participant "warnings"
participant "User"
"url_to_path()"->>"re": match netloc against pattern
alt netloc matches drive letter pattern
"url_to_path()"->>"warnings": warn about missing slash or drive letter
"warnings"->>"User": display UserWarning
else netloc is UNC path
"url_to_path()"->>"netloc": prepend UNC share notation
end
Class diagram for conditional Traversable import in helpers.pyclassDiagram
class helpers {
+_get_license_file() Traversable
}
class Traversable
helpers --> Traversable: returns
note for helpers "Traversable is imported from importlib.abc for Python <3.11, from importlib.resources.abc for Python >=3.11"
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
7c39afb to
8b97b70
Compare
7b26d90 to
4be6df2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `tests/packages/test_directory_dependency.py:99` </location>
<code_context>
expected = f"demo @ {path.as_uri()}"
- requirement = f"demo @ file://{path.as_posix()}"
+ prefix = "/" if sys.platform == "win32" else ""
+ requirement = f"demo @ file://{prefix}{path.as_posix()}"
_test_directory_dependency_pep_508("demo", path, requirement, expected)
</code_context>
<issue_to_address>
Test parametrization could improve readability and reduce duplication.
Consider using pytest parametrization to handle both Windows and non-Windows file URL prefixes, reducing repeated logic across tests.
</issue_to_address>
### Comment 2
<location> `tests/packages/test_file_dependency.py:88` </location>
<code_context>
expected = f"demo @ {path.as_uri()}"
- requirement = f"demo @ file://{path.as_posix()}"
+ prefix = "/" if sys.platform == "win32" else ""
+ requirement = f"demo @ file://{prefix}{path.as_posix()}"
_test_directory_dependency_pep_508("demo", path, requirement, expected)
</code_context>
<issue_to_address>
Consider adding tests for malformed file URLs (e.g., missing slashes, extra slashes, or invalid characters).
Adding tests for malformed or ambiguous file URLs will help verify error handling and improve reliability.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
4be6df2 to
bdd2f91
Compare
Summary by Sourcery
CI:
Summary by Sourcery
Ensure Python 3.14 compatibility by updating the CI workflow, refining Windows file URI handling with deprecation warnings, adjusting imports for Python version differences, and updating tests and integration scripts accordingly
Bug Fixes:
Enhancements:
CI:
Tests: