Skip to content

Use encoding option or binary mode for open()#9608

Merged
uranusjr merged 2 commits intopypa:masterfrom
methane:open-encoding
Feb 18, 2021
Merged

Use encoding option or binary mode for open()#9608
uranusjr merged 2 commits intopypa:masterfrom
methane:open-encoding

Conversation

@methane
Copy link
Copy Markdown
Contributor

@methane methane commented Feb 13, 2021

No description provided.

if requested:
requested_path = os.path.join(dest_info_dir, 'REQUESTED')
with open(requested_path, "w"):
with open(requested_path, "wb"):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

As this simply opens then closes (creating an empty file), this makes no real difference. I'm inclined to prefer not to use "b" because that implies the file is binary, and it isn't. But I don't really care much.

pathlib.touch might be better, now we don't have to worry about Python 2 compatibility.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I prefer binary mode because creating TextIOWrapper is waste of time/memory and using locale-specific encoding is root of bugs.
But I respect your preference.

Copy link
Copy Markdown
Member

@pfmoore pfmoore Feb 13, 2021

Choose a reason for hiding this comment

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

That's a good point I hadn't considered.

Actually, I went looking to see what encoding the REQUESTED file should use. https://packaging.python.org/specifications/recording-installed-packages/ says it's tool-specific, and refers to PEP 376 which says it can be empty or contain a comment. So the content is essentially irrelevant, and binary mode therefore makes sense.

Let's use binary mode, and add a comment saying "Create an empty file (see PEP 376)", just to be clear that the encoding isn't relevant.

@pfmoore pfmoore added the skip news Does not need a NEWS file entry (eg: trivial changes) label Feb 13, 2021
@uranusjr uranusjr merged commit 1783041 into pypa:master Feb 18, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

skip news Does not need a NEWS file entry (eg: trivial changes)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants