-
Notifications
You must be signed in to change notification settings - Fork 1.7k
py3: remove mock backport
#4507
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
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Summary: We now always import `unittest.mock` directly, rather than conditionally depending on the `@org_pythonhosted_mock` backport. This simplifies both the Python code and the BUILD structure, which was a bit precarious. Part of #4488. Test Plan: All tests pass here and in a test sync (<http://cl/348690501>). wchargin-branch: py3-no-mock-backport wchargin-source: 23e7a2ef6663dee18c778cf9b7bfa53a9182cbd7
wchargin-branch: py3-no-mock-backport wchargin-source: 6753d5dcc2d1f3fa908c207dab238d9928397cfd
wchargin
added a commit
that referenced
this pull request
Dec 28, 2020
Summary: Noticed in the original, failed CI run of #4507 that this script ran in Python 2, since its `print()` caused an extra `()` to be printed (since we turned off `print_function` in #4503. This happens because its shebang specifies the system’s default `python` and the CI job that invokes it doesn’t set up a Python 3 virtualenv. The script still works, since it’s effectively bicompatible, but for consistency we should still make sure that everything runs on Python 3 only (else the tasks in #4488 are not necessarily sound). Test Plan: `git grep bin/python` only matches this line. wchargin-branch: py3-whitespace-test wchargin-source: 2da3c38930b54c3881c39d2813a8686989f9430c
wchargin
added a commit
that referenced
this pull request
Dec 28, 2020
Summary: The `html.escape` function superseded `cgi.escape` in Python 3.2, prior to which the `html` module did not exist. We now unconditionally import `html`, without falling back to `cgi`. Part of #4488. Test Plan: Tested the Pip package in Jupyter. When rebased on top of #4507, which removes the `unittest.mock` backport, a `git grep -A1 '^try:'` shows that the remaining top-level conditional statements are okay: a bunch of TensorFlow compatibility incantations for summaries, and one conditional import of `botocore`. wchargin-branch: py3-html-escape wchargin-source: 2f17731ec538eb904d673edccf63310afa585ef3
wchargin
added a commit
that referenced
this pull request
Jan 5, 2021
Summary: The `html.escape` function superseded `cgi.escape` in Python 3.2, prior to which the `html` module did not exist. We now unconditionally import `html`, without falling back to `cgi`. Part of #4488. Test Plan: Tested the Pip package in Jupyter. When rebased on top of #4507, which removes the `unittest.mock` backport, a `git grep -A1 '^try:'` shows that the remaining top-level conditional statements are okay: a bunch of TensorFlow compatibility incantations for summaries, and one conditional import of `botocore`. wchargin-branch: py3-html-escape
psybuzz
approved these changes
Jan 5, 2021
Contributor
psybuzz
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.
Nice to see so many deletions!
wchargin
added a commit
that referenced
this pull request
Jan 5, 2021
Summary: Noticed in the original, failed CI run of #4507 that this script ran in Python 2, since its `print()` caused an extra `()` to be printed (since we turned off `print_function` in #4503). This happens because its shebang specifies the system’s default `python` and the CI job that invokes it doesn’t set up a Python 3 virtualenv. The script still works, since it’s effectively bicompatible, but for consistency we should still make sure that everything runs on Python 3 only (else the tasks in #4488 are not necessarily sound). Test Plan: `git grep bin/python` only matches this line, and we have no uses of `/usr/bin/env python`, either. wchargin-branch: py3-whitespace-test
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
We now always import
unittest.mockdirectly, rather than conditionallydepending on the
@org_pythonhosted_mockbackport. This simplifies boththe Python code and the BUILD structure, which was a bit precarious.
Part of #4488.
Test Plan:
All tests pass here and in a test sync (http://cl/348690501).
wchargin-branch: py3-no-mock-backport