bpo-30296 Remove unnecessary tuples, lists, sets, and dicts #1489

jdufresne · 2017-05-07T12:57:07Z

Replaced list() with list comprehension
Replaced dict() with dict comprehension
Replaced set() with set comprehension
Replaced builtin func() with func() when supported (e.g. any(), all(), tuple(), min(), &
max())

the-knights-who-say-ni · 2017-05-07T12:57:09Z

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA).

Unfortunately our records indicate you have not signed the CLA. For legal reasons we need you to sign this before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

Thanks again to your contribution and we look forward to looking at it!

rhettinger · 2017-05-07T16:34:51Z

Lib/_weakrefset.py

Let's use: set(map(ref, other)) for all of these.

rhettinger · 2017-05-07T16:36:41Z

Lib/distutils/msvc9compiler.py

Yes to this one. Set literals are always a win.

rhettinger · 2017-05-07T16:37:27Z

Lib/inspect.py

Yes, this one is fine.

I would keep the original code.

rhettinger · 2017-05-07T16:40:40Z

Lib/ntpath.py

This code smells bad (both the original and revision). Can this be replaced with any(...).

rhettinger · 2017-05-07T16:45:28Z

Lib/optparse.py

This looks like a reasonable change, but optparse is essentially defunct so we should leave it alone.

rhettinger · 2017-05-07T17:30:11Z

Tools/stringbench/stringbench.py

I prefer not to touch a benchmark file lest it cause comparability issues.

rhettinger · 2017-05-07T17:31:06Z

setup.py

Skip this. I prefer not to touch anything in setup.py.

rhettinger · 2017-05-07T17:31:23Z

setup.py

rhettinger · 2017-05-07T17:31:29Z

setup.py

rhettinger · 2017-05-07T17:31:48Z

setup.py

Ambivalent.

terryjreedy

I keep idlelib 3.6 and 3.7 in sync as much as possible.
Please remove all Lib/idlelib/multicall.py changes from this PR and optionally (and preferably) make the 2 approved changes in a separate PR that can be backported easily.

terryjreedy · 2017-05-12T20:38:51Z

Lib/idlelib/multicall.py

I prefer the change as easier to read and likely faster. Ditto for the other dict(list) to dict comp change in this file. (Agreeing with Raymond.)

terryjreedy · 2017-05-12T20:39:48Z

Lib/idlelib/multicall.py

Agree with Raymond, leave as is.

jdufresne · 2017-05-13T15:12:34Z

Thanks for the reviews! I believe I have addressed all feedback comments. If I missed or misunderstood any of the previous comments, please let me know.

* Replaced list(<generator expression>) with list comprehension * Replaced dict(<generator expression>) with dict comprehension * Replaced set(<list literal>) with set literal * Replaced builtin func(<list comprehension>) with func(<generator expression>) when supported (e.g. any(), all(), tuple(), min(), & max())

serhiy-storchaka · 2017-05-18T14:11:18Z

Lib/email/headerregistry.py

The original variant, with list comprehension, is faster than the variant with a generator. The difference is up to 2 times for simple tests.

I would keep the original code.

serhiy-storchaka · 2017-05-18T14:11:46Z

Lib/inspect.py

I would keep the original code.

serhiy-storchaka · 2017-05-18T14:15:08Z

Lib/logging/config.py

        props = config.pop('.', None)
        # Check for valid identifiers
-        kwargs = dict([(k, config[k]) for k in config if valid_ident(k)])
+        kwargs = dict((k, config[k]) for k in config if valid_ident(k))


I would use a dict comprehension.

And the same for other change in this file.

serhiy-storchaka · 2017-05-18T14:30:29Z

Lib/pstats.py

                # format used by cProfile
-                new_callers[func] = tuple([i[0] + i[1] for i in
-                                           zip(caller, new_callers[func])])
+                new_callers[func] = tuple(i[0] + i[1] for i in zip(caller, new_callers[func]))


I would keep the original code.

Maybe unpack a tuple:

tuple([i + j for i, j in zip(caller, new_callers[func])])

serhiy-storchaka · 2017-05-18T14:36:36Z

Lib/symtable.py

I would keep the original code.

serhiy-storchaka · 2017-05-18T14:39:24Z

Lib/turtle.py

I would keep the original code.

serhiy-storchaka · 2017-05-18T14:42:12Z

Lib/turtle.py

.keys() can be removed. Iterate just docsdict.

serhiy-storchaka · 2017-05-18T14:48:36Z

Lib/urllib/request.py

I would keep the original code.

serhiy-storchaka · 2017-05-18T14:52:09Z

Lib/urllib/request.py

        headers = dict(req.unredirected_hdrs)
-        headers.update(dict((k, v) for k, v in req.headers.items()
-                            if k not in headers))
+        headers.update((k, v) for k, v in req.headers.items() if k not in headers)


This changes the semantic. Updated headers is used in the generator expression. I don't know whether this is good.

I would use a dict comprehesion.

serhiy-storchaka · 2017-05-18T14:54:17Z

Tools/gdb/libpython.py

I would keep the original code.

the-knights-who-say-ni added the CLA not signed label May 7, 2017

jdufresne changed the title ~~Remove unnecessary tuples, lists, sets, and dicts~~ bpo-30296 Remove unnecessary tuples, lists, sets, and dicts May 7, 2017

rhettinger requested changes May 7, 2017

View reviewed changes

Mariatta removed the CLA not signed label May 8, 2017

the-knights-who-say-ni added the CLA signed label May 8, 2017

terryjreedy requested changes May 12, 2017

View reviewed changes

rhettinger approved these changes May 15, 2017

View reviewed changes

rhettinger merged commit 3972628 into python:master May 18, 2017

serhiy-storchaka reviewed May 18, 2017

View reviewed changes

serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this pull request Jul 7, 2017

bpo-30296: Partially revert python#1489.

d033141

jdufresne deleted the comprehensions branch November 18, 2017 17:14

Uh oh!

bpo-30296 Remove unnecessary tuples, lists, sets, and dicts #1489

bpo-30296 Remove unnecessary tuples, lists, sets, and dicts #1489

Uh oh!

Conversation

jdufresne commented May 7, 2017

Uh oh!

the-knights-who-say-ni commented May 7, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

terryjreedy left a comment

Choose a reason for hiding this comment

Uh oh!

terryjreedy May 12, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

terryjreedy May 12, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jdufresne commented May 13, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

terryjreedy May 12, 2017 •

edited

Loading

terryjreedy May 12, 2017 •

edited

Loading