Skip to content

Conversation

@srittau
Copy link
Collaborator

@srittau srittau commented May 19, 2021

This uses my original idea. Why this is currently doesn't work in corner cases due to a mypy bug, I think this is the best solution:

  • Is it correct.
  • It is very unlikely to be an issue in practice as this is an internal constructor, and the close_fileobj argument is most likely provided as a literal anyway.
  • The mypy bug is easily worked around by using # type: ignore or Literal[True, False] in case it comes up in practice.
  • We don't need to remember to fix this when mypy gets fixed.

@github-actions
Copy link
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

Copy link
Collaborator

@Akuli Akuli left a comment

Choose a reason for hiding this comment

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

This still feels unnecessarily complicated, but I like the correctness.

@Akuli
Copy link
Collaborator

Akuli commented May 19, 2021

CI failure seems unrelated

@srittau srittau merged commit 2c7bae6 into python:master May 19, 2021
@srittau srittau deleted the closable-zipfile branch May 19, 2021 18:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants