Skip to content

bpo-30803: clarify truth value testing documentation#2508

Merged
terryjreedy merged 3 commits into
python:masterfrom
peterthomassen:master
Jul 29, 2017
Merged

bpo-30803: clarify truth value testing documentation#2508
terryjreedy merged 3 commits into
python:masterfrom
peterthomassen:master

Conversation

@peterthomassen
Copy link
Copy Markdown
Contributor

@peterthomassen peterthomassen commented Jun 30, 2017

The documentation was unclear about the truth value of sets. This PR clarifies the situation, based on a phrasing proposal from https://bugs.python.org/issue30803.

https://bugs.python.org/issue30803

Comment thread Doc/library/stdtypes.rst Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • any empty set or mapping, for example, set(), {}.

Comment thread Doc/library/stdtypes.rst Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I much prefer a minimal change, adding "set or" and "set(), "

@ghost
Copy link
Copy Markdown

ghost commented Jul 3, 2017

This PR has both the "CLA signed" and the "CLA not signed" labels. Is it Schrödinger's CLA? ;)

@peterthomassen
Copy link
Copy Markdown
Contributor Author

peterthomassen commented Jul 10, 2017

@terryjreedy Well, I'm not sure what to do at this point. I adapted the proposal by @rhettinger who is also a very experienced Python contributor. I found this a very reasonable course of action.

Now you say that you "much prefer a minimal change" while putting forward zero arguments (not even a subjective one). On bpo, you also said very broadly that you would not merge this patch. It's true that patches don't need to be accepted, but it's also true that a community works better if a contribution is valued, and criticism is backed by an argument.

Other people may "much prefer" concise documentation. The previous phrasing was not concise, and I can't following the reasoning for keeping something just because the change would be minimal. In fact, one may argue that this is a good moment to rephrase the whole thing, as we're touching it anyways.

Comment thread Doc/library/stdtypes.rst Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I strongly prefer 'collection' rather than 'container'. Collections do not 'contain' python objects. They may contain references to objects, values corresponding to objects, or info needed to generate objects by algorithm.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That aside, Raymond's wording is clearer, simpler, and more complete than your rewrite. Use what he wrote.

Copy link
Copy Markdown
Contributor Author

@peterthomassen peterthomassen Jul 14, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see why one can't say 'container' even if the structure may 'contain' entities beyond Python objects. There is still something contained within; further, nobody claimed them to be 'object containers' only or made some other implicit restriction.

Similarly: Does a 'collection' only 'collect' Python objects? (Does it actually 'collect' anything?)

Anyhow, the docs use 'collection' and 'container' pretty much interchangeably, and I consider this point completely moot. However, you certainly have better understanding, so I used now what you strongly and wisely preferred, hence graciously commanded me to. I hope we now all agree on this matter.

Copy link
Copy Markdown
Member

@bitdancer bitdancer Jul 14, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@peterthomassen just FYI, 'collection' is technically correct here and 'container' is technically wrong, since the Container abc only requires __contains__ to be defined, which if nothing else was defined would make the object true. A Collection, on the other hand, is sized, and so can be false if empty.

You are right, however, that we do sometimes use them interchangeably. But we probably shouldn't :)

Thanks for taking the time to work through this PR.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bitdancer Thank you for sharing your knowledge. I understand your point (which is technical, not just semantic), and agree that as a consequence, 'empty container' is not a very good term if a Container type is not required to have __len__ defined. (As an aside, isinstance(set(), collections.abc.Container) is still true, and the same is the case for the other empty collections listed in the docs section this PR deals with.)

I guess it is not defined whether 'container' and 'collection' mean Container and Collection, respectively (but I don't feel this needs to be rectified).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, the lower case terms do not necessarily refer to the ABC definitions, but the closer we adhere to those definitions in the docs, the better, probably. The ABCs represent our formal definitions of the terms, as opposed to the colloquial usage.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am fussy about the terminology for the practical reason that I regularly see people confused by 'container', including myself as a student long ago when I realized that a set is not like a box or room but a pure conceptual grouping. The issue comes up at least yearly on python-list, where I have been active for over 20 years.

Copy link
Copy Markdown
Member

@terryjreedy terryjreedy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this version ;-).

@Mariatta Mariatta added needs backport to 3.6 docs Documentation in the Doc dir labels Jul 14, 2017
Comment thread Doc/library/stdtypes.rst

.. index:: single: true
* numeric zero of any type: ``0``, ``0.0``, ``0j``, ``Decimal(0)``,
``Fraction(0, 1)``
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What was wrong with the original wording? And aren’t Decimal and Fraction out of scope of “the built-in objects”?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the issue for 'wrong'. Decimal and Fraction are built-in in the sense of C-coded stdlib objects, but not in 'part of builtins' module. I could go either way with them.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I see that this wording probably comes from @terryjreedy’s suggestion https://bugs.python.org/issue30803#msg298099, although I don’t see any reasoning. IMO the current zero of any numeric type, or Raymond’s numbers equal to zero, are clearer.

Numeric zero of any type raises the question of what a numeric zero of the string type looks like. I think other languages consider strings with the digit zero to be false. (PHP or Perl perhaps?)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with changing back to 'zero of any numeric type'. Peter, change this (I cannot, for some reason) and I will merge.

@terryjreedy terryjreedy self-assigned this Jul 24, 2017
@python python deleted a comment from the-knights-who-say-ni Jul 24, 2017
@terryjreedy
Copy link
Copy Markdown
Member

Peter, I cannot add commits to this issue, and a few others. Please follow the instructions at https://help.github.com/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork/#enabling-repository-maintainer-permissions-on-existing-pull-requests to make sure "Allow edits from maintainers." is checked for this and other IDLE issues. Please let me know what you find. If this permission is already granted, then I have some other problem with github.

@peterthomassen
Copy link
Copy Markdown
Contributor Author

@terryjreedy Terry, I just checked that "allow edits" box.

@terryjreedy terryjreedy merged commit caa1280 into python:master Jul 29, 2017
terryjreedy pushed a commit to terryjreedy/cpython that referenced this pull request Jul 29, 2017
…2508)

Initial patch by Peter Thomassen.
(cherry picked from commit caa1280)
@bedevere-bot
Copy link
Copy Markdown

GH-2946 is a backport of this pull request to the 3.6 branch.

terryjreedy added a commit that referenced this pull request Jul 29, 2017
…2946)

Initial patch by Peter Thomassen.
(cherry picked from commit caa1280)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs Documentation in the Doc dir

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants