-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Disable unspecified-encoding for py-version above Python 3.15 #10800
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
base: main
Are you sure you want to change the base?
Conversation
Pierre-Sassoulas
left a comment
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.
Thank you for working on this !
pylint/checkers/stdlib.py
Outdated
| self.add_message( | ||
| "unspecified-encoding", node=node, confidence=confidence | ||
| ) | ||
| if emit_unspecified_encoding: | ||
| self.add_message( | ||
| "unspecified-encoding", node=node, confidence=confidence | ||
| ) | ||
|
|
||
| if encoding_arg: | ||
| encoding_arg = utils.safe_infer(encoding_arg) | ||
|
|
||
| if isinstance(encoding_arg, nodes.Const) and encoding_arg.value is None: | ||
| self.add_message( | ||
| "unspecified-encoding", node=node, confidence=confidence | ||
| ) | ||
| if emit_unspecified_encoding: | ||
| self.add_message( | ||
| "unspecified-encoding", node=node, confidence=confidence | ||
| ) |
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.
We'd already have exited from the function at this point, right ?
doc/user_guide/checkers/features.rst
Outdated
| It is better to specify an encoding when opening documents. Using the system | ||
| default implicitly can create problems on other operating systems. See | ||
| https://peps.python.org/pep-0597/ | ||
| This message is only emitted when ``py-version`` is lower than 3.15. |
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.
This documentation need to be added inside the code, as this file is generated.
| if sys.version_info < (3, 15): | ||
| assert mod_test._linter.is_message_enabled("unspecified-encoding") | ||
| else: | ||
| assert not mod_test._linter.is_message_enabled("unspecified-encoding") |
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.
What is the idea here ? Is this LLM suggested ? Because I don't see any code related to this in the implementation afaiu
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.
sorry this is not the final, I am moving it to draft, I took the initial idea from LLM, Thanks ! @Pierre-Sassoulas
|
Thanks for the fast review @Pierre-Sassoulas ! Once finished with the optimal version of the PR, I will request for re-review! Thanks again ! |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #10800 +/- ##
==========================================
- Coverage 95.98% 95.98% -0.01%
==========================================
Files 176 176
Lines 19564 19566 +2
==========================================
+ Hits 18779 18780 +1
- Misses 785 786 +1
🚀 New features to boost your workflow:
|
This comment has been minimized.
This comment has been minimized.
|
🤖 According to the primer, this change has no effect on the checked open source code. 🤖🎉 This comment was generated for commit a8b96df |
Type of Changes
Description
Closes #10791
This PR updates the behavior of the
W1514 (unspecified-encoding)warning to account forchanges introduced by PEP 686.
Starting with Python 3.15, UTF-8 becomes the default text encoding, making the absence of
an explicit
encoding=argument safe and predictable. As a result, emittingW1514bydefault for Python 3.15+ produces unnecessary false positives.
What this PR does
W1514enabled by default for Python versions < 3.15W1514by default for Python >= 3.15This reduces noise for users targeting modern Python versions while maintaining safety
checks for older environments.