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

louisom · 2017-04-05T05:08:06Z

No description provided.

mention-bot · 2017-04-05T05:08:08Z

@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 · 2017-04-08T06:10:54Z

Doc/glossary.rst

@@ -921,6 +927,11 @@ Glossary
      reserved for rare cases where there are large numbers of instances in a
      memory-critical application.

+   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 · 2017-04-08T06:17:40Z

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

louisom · 2017-04-10T04:17:53Z

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

orsenthil · 2017-04-10T04:20:12Z

Doc/glossary.rst

+   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 · 2017-04-10T05:51:47Z

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 · 2017-04-10T06:06:20Z

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

serhiy-storchaka · 2017-04-10T06:43:29Z

Doc/library/stdtypes.rst

-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 · 2017-04-10T06:44:56Z

Doc/library/stdtypes.rst

-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 · 2017-04-10T06:47:59Z

Doc/library/stdtypes.rst

+  ``{'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 · 2017-04-10T16:40:58Z

Doc/glossary.rst

@@ -272,6 +272,12 @@ Glossary
      keys can be any object with :meth:`__hash__` and :meth:`__eq__` methods.
      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 · 2017-04-10T16:42:14Z

Doc/glossary.rst

+   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 · 2017-04-10T16:44:28Z

Doc/library/stdtypes.rst

+* 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 · 2017-04-10T16:45:41Z

Doc/library/stdtypes.rst

+  ``{'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 · 2017-04-26T13:17:45Z

Doc/library/stdtypes.rst

 The constructors for both classes work the same:

 .. 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 · 2017-06-08T00:01:01Z

Doc/glossary.rst

@@ -272,6 +272,12 @@ Glossary
      keys can be any object with :meth:`__hash__` and :meth:`__eq__` methods.
      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 · 2017-06-08T00:01:44Z

Doc/glossary.rst

@@ -921,6 +927,12 @@ Glossary
      reserved for rare cases where there are large numbers of instances in a
      memory-critical application.

+   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 · 2017-06-08T00:09:28Z

Doc/library/stdtypes.rst

 The constructors for both classes work the same:

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

+   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 · 2017-06-08T00:09:32Z

Doc/library/stdtypes.rst

 The constructors for both classes work the same:

 .. 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 · 2017-06-08T00:17:55Z

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 · 2017-06-21T16:25:48Z

Doc/library/stdtypes.rst

+
+   * 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 · 2018-02-21T01:51:37Z

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 · 2018-05-21T04:29:14Z

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 needs backport to 3.9 only security fixes and removed needs backport to 3.6 labels May 21, 2020

Uh oh!

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

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

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!