-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Fix couple of EncodingWarnings #10954
Conversation
We're not supposed to diverge here, but make this change to fix an unavoidable EncodingWarning that is otherwise raised in pytest's test suite. The behavior should be exactly the same besides the warning, hopefully that won't cause confusion.
for more information, see https://pre-commit.ci
@@ -953,7 +953,7 @@ def ensure(self, *args, **kwargs): | |||
else: | |||
p.dirpath()._ensuredirs() | |||
if not p.check(file=1): | |||
p.open("w").close() | |||
p.open("wb").close() |
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.
To just touch the file, pathlib uses the below pattern -- this might be a larger change than wanted, though:
p.open("wb").close() | |
os.close(os.open(self.strpath, flags=os.O_CREAT)) |
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 code Was strictly copied in to enable drop of the py dep, so let's go for minimal changes instead of nice ones as we want to shed it as soon as possible
Thanks for fixing this! I got the warning in my CI configuration, any idea when a new stable release will be tagged? |
See #10328 (comment) for context.
The
py.path
one might be better to leave as-is, but I lean toward fixing.