py3: excise `six` #4510

wchargin · 2020-12-24T01:34:33Z

Armed with a handful of eldritch macros and more patience than Python
deserves, we cast six out into the void.

This is a mixture of automated and manual changes. See individual
commits for details. Plan to merge with rebase.

Test sync passes: http://cl/348866797.

Part of #4488.

nfelt

Thanks for doing this! 🧹🧹🧹 Those vim macros are indeed scary...

nfelt · 2021-01-07T18:32:00Z

tensorboard/compat/tensorflow_stub/io/gfile.py

    def glob(self, filename):
        """Returns a list of files that match the given pattern(s)."""
-        if isinstance(filename, six.string_types):
+        if isinstance(filename, (str,)):


It looks like in every* place that we had used six.string_types we're just passing it into isinstance(), so it seems better to just replace it with str rather than (str,) since the latter is entirely vestigial?

* The one exception being in a docstring, but in that case I think str would also be more intelligible than (str,)

Good point. Done.

@q

Summary: `six.assertRaisesRegex(self, ...) -> self.assertRaisesRegex(...)`; `six.assertCountEqual(self, ...) -> self.assertCountEqual(...)`. Generated manually (with Vim macros), since the `self` transposition means that a blanket `sed` doesn’t suffice. Tried using Comby, Codemod, and Semgrep, but none of them could handle this enormously complicated pattern (Comby has bad Python-whitespace support, Codemod is plain regex only, and Semgrep just spit out bad output). Incantation (requires `vim-fugitive` for `Ggrep`): ``` vim +'let @q = "cwself\<Esc>/self\rdf," | let @w = "@q:noau w|cn\<CR>@w" | Ggrep "six\.$assertRaisesRegex\|assertCountEqual$" | normal @w' +q && flake8 tensorboard/ --select=F401 | cut -d : -f 1 | xargs git sed '/import six/d' && black tensorboard ``` Test Plan: Suffices that all tests pass. wchargin-branch: py3-assertraisesregex

@q

Summary: `six.iteritems(...) -> (...).items()`, and likewise for `itervalues` and `iterkeys`. Incantation (requires `vim-fugitive` for `Ggrep`): ``` vim +'let @q = "df.4x\"zdw%a.\<C-R>z()\<Esc>bbhhdsb" | let @w = "@q:noau w|cn\<CR>@w" | Ggrep 'six\.iter' | normal @w' +q && flake8 tensorboard/ --select=F401 | cut -d : -f 1 | xargs git sed '/import six/d' && black tensorboard ``` Test Plan: It suffices that all tests pass. wchargin-branch: py3-six-iter-helpers

@q

Summary: `@six.add_metaclass(M) class C(object) -> class C(metaclass=M)`. Incantation (requires `vim-fugitive` for `Ggrep`): ``` vim +'let @q = "f(\"zdibdd0f(cibmetaclass=\<C-r>z\<Esc>" | let @w = "@q:noau w|cn\<CR>@w" | Ggrep '@six.add_metaclass' | normal @w' +q && flake8 tensorboard/ --select=F401 | cut -d : -f 1 | xargs git sed '/import six/d' && black tensorboard ``` Test Plan: It suffices that all tests pass. wchargin-branch: py3-six-add-metaclass

Summary: We use `six.moves` mostly for `xrange`, but also `urllib`, and occasionally `queue` or `input`. Upgrading `urllib` is context-sensitive because `six.moves.urllib` always has all submodules, even though `urllib` may not: ```python >>> import urllib >>> urllib.request Traceback (most recent call last): File "<stdin>", line 1, in <module> AttributeError: module 'urllib' has no attribute 'request' >>> from six.moves import urllib >>> urllib.request <module 'six.moves.urllib.request'> ``` We make those moves here, and also clean up the rest of the long tail such that the remaining moves for `xrange` are all mechanical and can be fixed in a follow-up commit. Test Plan: It suffices that tests pass (but they really have to run; `flake8` does not suffice here). Remaining: ``` $ git grep -h 'six.*moves' | sort -u from six.moves import xrange from six.moves import xrange # pylint: disable=redefined-builtin ``` wchargin-branch: py3-six-non-xrange-moves

Summary: Generated with: ``` git sed '/six.*moves/d' && git sed s/xrange/range/ && black tensorboard ``` where [`git-sed`] does what it says on the tin. [`git-sed`]: https://gist.github.com/wchargin/ea868384294e26103b90ac42e0de82d9 Test Plan: It suffices that all tests pass. wchargin-branch: py3-six-moves-xrange

Summary: Generated with: ``` git sed 's/_\?six\.binary_type/bytes/g' && git sed 's/_\?six\.text_type/str/g' && git sed 's/_\?six\.string_types/str/g' && git sed 's/_\?six\.b($"[^"]*"$)/b\1/g' && flake8 tensorboard/ --select=F401 | cut -d : -f 1 | xargs git sed '/import six/d' && black tensorboard ``` where [`git-sed`] does what it says on the tin. The `b("foo")` regex suffices because all of our calls to `six.b` are with string literals short enough to fit on one line. The `_\?`s are because some modules import `six` as `_six`. The `string_types` to `str` replacement is sound even though `string_types` was actually defined as `(str,)` because they’re equivalent in all actual call sites (usually `isinstance`s). [`git-sed`]: https://gist.github.com/wchargin/ea868384294e26103b90ac42e0de82d9 Test Plan: It suffices that all tests pass. wchargin-branch: py3-six-text-bytes

Summary: Generated with `git sed 's/six\.$StringIO\|BytesIO$/io.\1/g'` and then manually fixing imports in affected files, where [`git-sed`] does what it says on the tin. Manual updates needed to `scalars_plugin.py` and its tests, since they didn’t follow the “only import modules” style rule and thus didn’t match the `sed` expression. [`git-sed`]: https://gist.github.com/wchargin/ea868384294e26103b90ac42e0de82d9 Test Plan: It suffices that all tests pass. wchargin-branch: py3-six-stringio-bytesio

Summary: A few long-tail stragglers, manually updated. We still have the `six` dependency in our codebase because protobuf requires it to exist(…), but all of our code is now six-free. Test Plan: All tests pass, and `git grep '$import\|from$ six'` fails. wchargin-branch: py3-six-misc

Summary: As of the previous sequence of changes, none of our code uses `six`. We still keep it in the build workspace because protobuf requires that it exist there (see comment in `third_party/python.bzl`), but we can remove all dependency edges from our code to `@org_pythonhosted_six`. Generated with: ``` buildozer //tensorboard/...:all 'remove deps @org_pythonhosted_six' ``` Test Plan: It suffices that all tests pass both here and in a test sync. wchargin-branch: py3-no-six-deps

Summary: As of #4510, we don’t need `six`, so we can remove it from the Pip package requirements. Part of #4488. Test Plan: Running `:extract_pip_package` and installing it into a virtualenv still installs `six` as a transitive dep, so this is actually no functional change for now. Maybe one day. wchargin-branch: py3-norequire-six wchargin-source: c7a05be7531841c2c998bf06ceb3501c2a80f16a

Summary: As of #4510, we don’t need `six`, so we can remove it from the Pip package requirements. Part of #4488. Test Plan: Running `:extract_pip_package` and installing it into a virtualenv still installs `six` as a transitive dep, so this is actually no functional change for now. Maybe one day. wchargin-branch: py3-norequire-six

wchargin added core:backend type:cleanup labels Dec 24, 2020

This comment has been minimized.

Sign in to view

google-cla bot added the cla: yes label Dec 24, 2020

wchargin mentioned this pull request Dec 24, 2020

Replace py2isms with py3 #4488

Open

10 tasks

wchargin requested a review from nfelt December 24, 2020 01:54

nfelt approved these changes Jan 7, 2021

View reviewed changes

wchargin force-pushed the wchargin-py3-excise-six branch from 7ee0ce8 to 67d53c8 Compare January 7, 2021 19:33

wchargin added 9 commits January 7, 2021 16:11

wchargin force-pushed the wchargin-py3-excise-six branch from 67d53c8 to 4d66f55 Compare January 8, 2021 00:21

wchargin merged commit 2fd4a35 into master Jan 8, 2021

wchargin deleted the wchargin-py3-excise-six branch January 8, 2021 18:39

wchargin mentioned this pull request Jan 12, 2021

py3: remove six from requirements #4536

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

py3: excise `six` #4510

py3: excise `six` #4510

Uh oh!

wchargin commented Dec 24, 2020 •

edited

Loading

Uh oh!

This comment has been minimized.

nfelt left a comment

Uh oh!

nfelt Jan 7, 2021

Uh oh!

wchargin Jan 7, 2021

Uh oh!

Uh oh!

py3: excise six #4510

py3: excise six #4510

Uh oh!

Conversation

wchargin commented Dec 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

nfelt left a comment

Choose a reason for hiding this comment

Uh oh!

nfelt Jan 7, 2021

Choose a reason for hiding this comment

Uh oh!

wchargin Jan 7, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

py3: excise `six` #4510

py3: excise `six` #4510

wchargin commented Dec 24, 2020 •

edited

Loading