[issue30296] Remove unnecessary tuples, lists, sets, and dicts from Lib

[Serhiy Storchaka]

Change by Serhiy Storchaka :


--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30296] Remove unnecessary tuples, lists, sets, and dicts from Lib

[miss-islington]

miss-islington  added the comment:


New changeset f8a3485dcd41a00c5e99c5be6adc67cb2638b366 by Miss Islington (bot) 
in branch '3.7':
Revert unneccessary changes made in bpo-30296 and apply other improvements. 
(GH-2624)
https://github.com/python/cpython/commit/f8a3485dcd41a00c5e99c5be6adc67cb2638b366


--
nosy: +miss-islington

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30296] Remove unnecessary tuples, lists, sets, and dicts from Lib

[miss-islington]

Change by miss-islington :


--
keywords: +patch
pull_requests: +5678

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30296] Remove unnecessary tuples, lists, sets, and dicts from Lib

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:


New changeset 3f2e6f15d64d81633b1fc0b308afc0d6e9026b61 by Serhiy Storchaka in 
branch 'master':
Revert unneccessary changes made in bpo-30296 and apply other improvements. 
(GH-2624)
https://github.com/python/cpython/commit/3f2e6f15d64d81633b1fc0b308afc0d6e9026b61


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30296] Remove unnecessary tuples, lists, sets, and dicts from Lib

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

What will you say Raymond?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30296] Remove unnecessary tuples, lists, sets, and dicts from Lib

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

> Why do you think that adding an unnecessary list comprehension around tuple() 
> call makes them better?

I just restore the original code because I don't think the changes make it 
better.

> Also, I don't think you should be able to override and revert all the 
> judgments I made when carefully reviewing all the OP's PR.  You are NOT the 
> sole arbiter of what goes into Python.

You asked my review and vetoing, and assigned this issue to me. I started 
reviewing, but when I finished it I found that the PR already is merged.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30296] Remove unnecessary tuples, lists, sets, and dicts from Lib

[Raymond Hettinger]

Raymond Hettinger added the comment:

Why do you think that adding an unnecessary list comprehension around tuple() 
call makes them better?

Also, I don't think you should be able to override and revert all the judgments 
I made when carefully reviewing all the OP's PR.  You are NOT the sole arbiter 
of what goes into Python.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30296] Remove unnecessary tuples, lists, sets, and dicts from Lib

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Since some changes looks worse than the old code to me (see my comments on 
GitHub), I have created PR 2624 which reverts changes with which I disagree or 
applies alternate changes.

--
resolution: fixed -> 
stage: resolved -> patch review
status: closed -> open

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30296] Remove unnecessary tuples, lists, sets, and dicts from Lib

[Serhiy Storchaka]

Changes by Serhiy Storchaka :


--
pull_requests: +2690

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30296] Remove unnecessary tuples, lists, sets, and dicts from Lib

[Terry J. Reedy]

Terry J. Reedy added the comment:

I made a note of the idlelib.multicall changes for when I review the module to 
modernize and test.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30296] Remove unnecessary tuples, lists, sets, and dicts from Lib

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Oh, the PR was merged while I made a review.

tuple([i for i in a]) is 2 times faster than tuple(i for i in a). Since there 
is no significant difference in readability I would keep the variant with a 
list comprehension if it is initial.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30296] Remove unnecessary tuples, lists, sets, and dicts from Lib

[Raymond Hettinger]

Changes by Raymond Hettinger :


--
resolution:  -> fixed
stage:  -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30296] Remove unnecessary tuples, lists, sets, and dicts from Lib

[Raymond Hettinger]

Raymond Hettinger added the comment:


New changeset 3972628de3d569c88451a2a176a1c94d8822b8a6 by Raymond Hettinger 
(Jon Dufresne) in branch 'master':
bpo-30296 Remove unnecessary tuples, lists, sets, and dicts (#1489)
https://github.com/python/cpython/commit/3972628de3d569c88451a2a176a1c94d8822b8a6


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30296] Remove unnecessary tuples, lists, sets, and dicts from Lib

[Raymond Hettinger]

Raymond Hettinger added the comment:

Serhiy, the latest version of this PR looks okay to me.  Do you care to veto 
any of the remaining suggested changes?

--
assignee: rhettinger -> serhiy.storchaka

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30296] Remove unnecessary tuples, lists, sets, and dicts from Lib

[Raymond Hettinger]

Raymond Hettinger added the comment:

Yes, please do keep any proposed idlelib changes separate.  Also, I don't any 
of these to be backported (it is stylistic and not about bugs).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30296] Remove unnecessary tuples, lists, sets, and dicts from Lib

[Terry J. Reedy]

Terry J. Reedy added the comment:

As I said on review, I want idlelib changes either not made or in a separate 
patch that can be backported as is.  Partial backport/cherry-picks are a major 
nuisance.  I don't know if anything else should be treated similarly.

--
nosy: +terry.reedy

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30296] Remove unnecessary tuples, lists, sets, and dicts from Lib

[Raymond Hettinger]

Raymond Hettinger added the comment:

Jon, we usually try to avoid sweeping changes across the library and instead 
preference to focus on one module at a time (Guido calls this "holistic 
refactoring").  There is a risk of introducing bugs and of destabiliizing code 
that has sat untouched but working for a long time.

Also, as Serhiy and Steven have pointed out, there are sometimes subtle reasons 
to prevent the existing code.  For example, I find the original to be clearer 
in this example:

 - return list(set(item.name for item in self.list)) +
 + return list({item.name for item in self.list})

and the original to be both clearer and faster in this example:

 - return list(map(_convert, node.elts))
 + return [_convert(v) for v in node.elts]

In some cases, using "dict(...)" or "set(...)" is clearer about the intension 
or more readable because it is parallel to the surrounding code (such as the 
parallelism of successive line in ast.py).

In some cases such as _pydecimal we have a aversion to changing code that 
aspires to be backwards compatible across many versions.

That said, there are a few places where it might be worthwhile for making a 
change.  Converting to set literals is almost always a win:

 -interesting = set(("include", "lib", "libpath", "path"))   
 +interesting = {"include", "lib", "libpath", "path"}

As Serhiy noted, in some places using map() would be an improvement, so we 
should look at doing a few of those (where there is a win in term of speed, 
space, and readability):

 - version_info = tuple([int(x) for x in version.split(".")])
 + version_info = tuple(map(int, version.split(".")))

Overall, I think most of these shouldn't be done, but I will mark a few where 
some change might be considered an improvement.  Steven or Serhiy are both 
welcome to veto any one of those.  Likewise, if the original author of the code 
is still active, they have a veto as well.  The status quo usually wins until 
we're refactoring a single module and make improvements while thinking about 
that module.

You picked a difficult first patch because you picked a topic that we usually 
avoid (wholesale sweeps through the standard library) and an area where 
deciding "what is best" involves subtleties with respect to clarity, 
parallelism with surrounding code, speed, clear intention, back compatibility, 
and how the original author thinks about the code.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30296] Remove unnecessary tuples, lists, sets, and dicts from Lib

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

* Some functions that accept an arbitrary iterable first convert it to 
temporary list and iterate that list. In these cases there is no significant 
difference in memory usage between list comprehension and generator, but the 
variant with list comprehension is slightly faster.

* foo(bar(x) for x in a) can be written also as foo(map(bar, a)). Both 
expressions are idiomatic, different users have different preferences, the 
variant with map can be faster for larger loops. The variant with map can even 
be faster than comprehensions.

* Proposed patch changes some files which should be kept compatible with old 
Python versions. Old Python versions can not support set and dict 
comprehensions.

* In general we avoid making changes to many files that doesn't have obvious 
positive effect. Most changes doesn't have measurable effect, they are just 
style changes. But different users have different preferences and existing code 
is not obviously bad.

Assigned to Raymond because it usually opposed to such kind of changes but made 
very similar changes in issue22823.

--
assignee:  -> rhettinger
nosy: +rhettinger, serhiy.storchaka

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30296] Remove unnecessary tuples, lists, sets, and dicts from Lib

[Steven D'Aprano]

Steven D'Aprano added the comment:

Is this unnecessary code churn?

That's not a rhetorical question. Fixing code that isn't broken is not always a 
good idea.

``func()`` is not always identical to ``func()``, there are situations where there is a significant performance 
difference between the two. E.g. str.join() is significantly faster when passed 
a list than when passed a generator expression. According to my tests:

''.join(['abc' for x in range(100)])

is about 40% faster than

''.join('abc' for x in range(100))

--
nosy: +steven.daprano

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue30296] Remove unnecessary tuples, lists, sets, and dicts from Lib

[Jon Dufresne]

New submission from Jon Dufresne:

Lib has some patterns that could be easily discovered and cleaned up. Doing so 
will reduce the number of unnecessary temporary lists in memory and unnecessary 
function calls. It will also take advantage of Python's own rich features in a 
way that better dog foods the language. For example:

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

--
components: Library (Lib)
messages: 293190
nosy: jdufresne
priority: normal
pull_requests: 1591
severity: normal
status: open
title: Remove unnecessary tuples, lists, sets, and dicts from Lib
versions: Python 3.7

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com