bpo-29981: Update Index for set, dict, and generator 'comprehensions' by louisom · Pull Request #995 · python/cpython

louisom · Apr 5, 2017

No description provided.

mention-bot · Apr 5, 2017

@grapherd, thanks for your PR! By analyzing the history of the files in this pull request, we identified @birkenfeld, @vadmium and @benjaminp to be potential reviewers.

orsenthil

This change looks good to me.

serhiy-storchaka · Apr 8, 2017


+   set comprehension
+      A compact way to process all or part of the elements in a sequence and
+      return a set with the results. ``results = {'foo' for _ in range(10)}``


This is bad example. The result is just {'foo'}.

See an example in tutorial.

@serhiy-storchaka thanks for mention, change another example for set comprehension.

serhiy-storchaka · Apr 8, 2017

It may be worth to mention set and dict comprehensions as ways of creating sets and dicts in stdtypes.rst.

louisom · Apr 10, 2017

@serhiy-storchaka there are already mention set and dict omprehensions in stdtypes.rst, but not like link comprehension using one section.

orsenthil · Apr 10, 2017

+   set comprehension
+      A compact way to process all or part of the elements in a sequence and
+      return a set with the results. ``results = {c for c in 'abracadabra' if
+      c not in 'abc'`` generates a set of strings with ``{'r', 'd'}``.


This is missing }

serhiy-storchaka · Apr 10, 2017

I can't find mentions of set and dict comprehensions in stdtypes.rst.

A list comprehension is mentioned as the one of three ways of creating lists (in addition to the constructor and the bracket construction). But for sets and dicts only two ways of creating are mentioned (the constructor and the brace construction).

louisom · Apr 10, 2017

I mistake the outcome, it had written in tutorial/datastructure.rst, not stdtypes.rst

serhiy-storchaka · Apr 10, 2017

-Non-empty sets (not frozensets) can be created by placing a comma-separated list
-of elements within braces, for example: ``{'jack', 'sjoerd'}``, in addition to the
-:class:`set` constructor.
+Non-empty sets (not frozensets) canb be created by several ways:


Typo: canb.

serhiy-storchaka · Apr 10, 2017

-Non-empty sets (not frozensets) can be created by placing a comma-separated list
-of elements within braces, for example: ``{'jack', 'sjoerd'}``, in addition to the
-:class:`set` constructor.
+Non-empty sets (not frozensets) canb be created by several ways:


Empty sets also can be created by using the constructor and set comprehension.

I think empty set can't created by set comprehension, {} return a dict, I'll add up the constructor.

{x for x in ()}

We can't say that the set created by a set comprehension is non-empty until we have created it.

Great, I didn't thought that way to create a set before.

serhiy-storchaka · Apr 10, 2017

+  ``{'jack': 4098, 'sjoerd': 4127}`` or ``{4098: 'jack', 4127: 'sjoerd'}``
+* Using :class:`dict` constructor: ``dict()``,
+  ``dict([('foo', 100), ('bar', 200)])``, ``dict(foo=100, bar=200)``
+* Using dict comprehension: ``{x: x ** 2 for x in range(10)}``


Maybe "Using a dict comprehension"?

Another question, so {} is a dict comprehension, or a dict constructor?

{} is a dict display.

bitdancer

Thanks for working on this. I have some suggestions.

bitdancer · Apr 10, 2017

      Called a hash in Perl.

+   dictionary comprehension
+      A compact way to process all or part of the pair elements in a sequence


"process all or part of the pair elements in a sequence" makes no sense to me in this context. Most likely you just want "to process all or part of the items in a sequence" as with the set comprehension.

I think it would be good include a link to the reference documentation.

bitdancer · Apr 10, 2017

+   set comprehension
+      A compact way to process all or part of the elements in a sequence and
+      return a set with the results. ``results = {c for c in 'abracadabra' if
+      c not in 'abc'}`` generates a set of strings with ``{'r', 'd'}``.


I think you mean "generates the set of strings {'r', 'd'}.

I think it would be good to include a link to the reference documentation.

bitdancer · Apr 10, 2017

+* Using :class:`set` constructor: ``set()``, ``set('foobar')``,
+  ``set(['a', 'b', 'foo'])``
+* Using a set comprehension: ``{x for x in ()}``,
+  ``{c for c in 'abracadabra' if c not in 'abc'}``


Given that you adding this paragraph in parallel to the way it is done in list and tuple, it would be best to put it at the beginning of the section, the way it is in list and tuple. I would drop the example of creating the empty set via comprehension, it will just confuse the reader. Please also reorder the sentences so that they are in an order parallel to that used in list and tuple (ie: the constructor sentence is last). This is mostly a trivial style nit, but consistency is good :)

I move this into class dict, not sure if you are saying this section. But it is now consistent with list and tuple.

Remove empty set comprehension

bitdancer · Apr 10, 2017

+  ``{'jack': 4098, 'sjoerd': 4127}`` or ``{4098: 'jack', 4127: 'sjoerd'}``
+* Using :class:`dict` constructor: ``dict()``,
+  ``dict([('foo', 100), ('bar', 200)])``, ``dict(foo=100, bar=200)``
+* Using a dict comprehension: ``{}``, ``{x: x ** 2 for x in range(10)}``


As above, I think this would be better at the start of the section, with the constructor sentence last.

bitdancer

With this exception of the one typo, this looks good to me.

bitdancer · Apr 26, 2017

 .. class:: set([iterable])
           frozenset([iterable])

+   Sets can be created by several ways:b


Looks like you have a stray 'b' there at the end of the sentence.

Yes, I should self-check this one.

terryjreedy · Jun 8, 2017

      Called a hash in Perl.

+   dictionary comprehension
+      A compact way to process all or part of the items in a sequence


A list/set/dict comprehension processes the items in an iterable.
If the entry for listcomp says 'sequence', it should be changed. I also do not like the 'all or part of' phrase. The iteration runs to completion. If one wants to not run the iteration to completion, one must use an explicit for loop. Perhaps the 'or part' refers to 'if' clauses. But that is a form of processing. If the iterable yields volatile items, they are gone.

terryjreedy · Jun 8, 2017


+   set comprehension
+      A compact way to process all or part of the elements in a sequence and
+      return a set with the results. ``results = {c for c in 'abracadabra' if


terryjreedy · Jun 8, 2017


+   Sets can be created by several ways:
+
+   * Using a comma-separated list of elements within braces: ``{'jack', 'sjoerd'}``


I would use 'Use' instead of 'Using' here and rest of list

terryjreedy · Jun 8, 2017

 .. class:: set([iterable])
           frozenset([iterable])

+   Sets can be created by several ways:


'by several ways' is not idiomatic. Either 'by several means' or 'in several ways'. The latter is used below for dicts.

terryjreedy · Jun 8, 2017

Have any of the other reviewers downloaded and applied the patch and rebuilt the doc to check the changes, especially the new index entries?

berkerpeksag · Jun 21, 2017

+
+   * Using a comma-separated list of elements within braces: ``{'jack', 'sjoerd'}``
+   * Using a set comprehension: ``{c for c in 'abracadabra' if c not in 'abc'}``
+   * Using the type constructor: ``set()``, ``set('foobar')``, ``set(['a', 'b', 'foo'])``


I'm wondering if we should still advertise the use of set([...]). We replaced all instances of it with set literals in the stdlib. See http://bugs.python.org/issue22823 for details.

IMHO these examples are not for creating set literals. They demonstrate creating sets from iterables using the type constructor. Without these examples the user can though that {x for x in iterable} is the only way.

brettcannon · Feb 21, 2018

To try and help move older pull requests forward, we are going through and backfilling 'awaiting' labels on pull requests that are lacking the label. Based on the current reviews, the best we can tell in an automated fashion is that a core developer requested changes to be made to this pull request.

If/when the requested changes have been made, please leave a comment that says, I have made the requested changes; please review again. That will trigger a bot to flag this pull request as ready for a follow-up review.

willingc · May 21, 2018

Thanks @louisom for submitting a PR and @serhiy-storchaka, @orsenthil, @terryjreedy, @bitdancer, and @berkerpeksag for the reviews.

To try and help move older pull requests forward, I'm closing this PR since it has been languishing for almost a year.

bpo-29981: Update Index for set, dict, and generator 'comprehensions'

9c78379

the-knights-who-say-ni added the CLA signed label Apr 5, 2017

orsenthil approved these changes Apr 8, 2017

View reviewed changes

orsenthil requested a review from serhiy-storchaka April 8, 2017 05:54

serhiy-storchaka reviewed Apr 8, 2017

View reviewed changes

Addressed shrhiy-storchaka problem

f076f00

orsenthil reviewed Apr 10, 2017

View reviewed changes

louisom added 2 commits April 10, 2017 14:23

Fix typo

1337ee8

Addressed from serhiy-storchaka

d9e7c16

serhiy-storchaka reviewed Apr 10, 2017

View reviewed changes

Fix dict and set comprehension

5e4cbfa

bitdancer requested changes Apr 10, 2017

View reviewed changes

Addressed bitdancer problem

e909365

bitdancer requested changes Apr 26, 2017

View reviewed changes

Remove trailing character

073256a

bitdancer approved these changes Apr 26, 2017

View reviewed changes

Mariatta added docs Documentation in the Doc dir needs backport to 3.6 labels Jun 7, 2017

terryjreedy requested changes Jun 8, 2017

View reviewed changes

berkerpeksag reviewed Jun 21, 2017

View reviewed changes

brettcannon added the awaiting changes label Feb 21, 2018

willingc closed this May 21, 2018

DahlitzFlorian mentioned this pull request May 20, 2020

bpo-29981: Update Index for set, dict, and generator 'comprehensions' #20272

Merged

terryjreedy self-assigned this May 21, 2020

terryjreedy added needs backport to 3.7 and removed needs backport to 3.6 labels May 21, 2020


		Sets can be created by several ways:

		* Using a comma-separated list of elements within braces: ``{'jack', 'sjoerd'}``

Search code, repositories, users, issues, pull requests...

Uh oh!

Conversation

louisom commented Apr 5, 2017

Uh oh!

mention-bot commented Apr 5, 2017

Uh oh!

orsenthil left a comment

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!

serhiy-storchaka commented Apr 8, 2017

Uh oh!

louisom commented Apr 10, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

serhiy-storchaka commented Apr 10, 2017

Uh oh!

louisom commented Apr 10, 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!

bitdancer left a comment

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!

bitdancer left a comment

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 commented Jun 8, 2017

Uh oh!