classification
Title: distutils strtobool returns 0 and 1 rather than False and True
Type: behavior Stage: resolved
Components: Distutils Versions: Python 3.6
process
Status: closed Resolution: wont fix
Dependencies: Superseder:
Assigned To: Nosy List: Joseph Bane, Nathaniel Manista, SilentGhost, berker.peksag, dstufft, eric.araujo, ned.deily, rhettinger
Priority: normal Keywords: patch

Created on 2016-08-09 19:35 by Joseph Bane, last changed 2020-06-22 12:20 by Nathaniel Manista. This issue is now closed.

Files
File name Uploaded Description Edit
patch.diff Joseph Bane, 2016-08-09 19:35 strtobool return type updates
patch.diff Joseph Bane, 2016-08-09 20:12 Updated patch to remove casefold and apply to 3.6 review
Messages (8)
msg272263 - (view) Author: Joseph Bane (Joseph Bane) * Date: 2016-08-09 19:35
The distutils strtobool function returns 0 or 1 rather than the native boolean type True or False values:

https://hg.python.org/cpython/file/3.5/Lib/distutils/util.py#l304

Please see the attached patch for updates to this function. I have included updates to use a f-string in the raise and casefold instead of lower.
msg272265 - (view) Author: SilentGhost (SilentGhost) * (Python triager) Date: 2016-08-09 19:57
Couple of things: your patch doesn't seem to apply cleanly for whatever reason (most likely because it's a patch against python2); the change from lowercasing to casefolding doesn't seem justified to me; the formatted value needs to be repr-formatted - though again - a change that's beyond the scope of this issue.

The tests are unaffected by this change, however.
msg272266 - (view) Author: Joseph Bane (Joseph Bane) * Date: 2016-08-09 20:12
Thank you for the feedback. Please find an updated patch attached.
msg272267 - (view) Author: SilentGhost (SilentGhost) * (Python triager) Date: 2016-08-09 20:37
LGTM
msg272282 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2016-08-09 23:55
Could you also update documentation in Doc/distutils/apiref.rst?

Also, test_strtobool in Lib/distutils/tests/test_util.py doesn't test the following case:

    >>> from distutils.util import strtobool
    >>> strtobool('x')
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "/home/berker/projects/cpython/default/Lib/distutils/util.py", line 317, in strtobool
        raise ValueError("invalid truth value %r" % (val,)) 
    ValueError: invalid truth value 'x'

Bonus points if you add a test for that :)

Thanks!
msg272284 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2016-08-10 00:56
Joseph, thanks for taking an interest in improving Python and submitting this patch.  Alas, I don't think we should apply it.  There are always tradeoffs that have to be considered when changing existing code.  Distutils is one of the oldest and most external facing of Python's standard library modules.  Distutils is a key dependency of many other critical third-party projects, like setuptools and pip. Also, Numpy subclasses and extends parts of Distutils.  Many third-party projects are set up to work with multiple versions of Python so we always need to be careful about introducing changes in one version that might break that version independence.  On the one hand, this change is small and seems to be likely to be safe, although it is very difficult to know for sure how third-parties may currently be using strtobool() in their own code.  On the other hand, what does making this change buy us and users?  The only thing I can see is a more modern return type and slightly cleaner code.  In cases like that, we usually say that the status quo wins: we choose to not make changes that have no real benefit (other than a minor aesthetic one) but could possibly introduce an incompatibility.  Yes, if we were writing Distutils today, it would definitely return a boolean but that's not a good enough reason to change it now.  Other opinions?
msg272331 - (view) Author: Joseph Bane (Joseph Bane) * Date: 2016-08-10 13:27
Thank you for your feedback. I definitely understand your reasoning and commitment to not introducing breaking changes in externally facing codes. I also agree that this does fall slightly more on the side of aesthetic rather than functional changes. However, this change doesn't seem like it is significantly altering the interface of strtobool; if anything it is making it clearer and less surprising, especially for Python 2/3+ users.

I know, as someone in that category, having learned Python at version 2.5, that it was a bit surprising when I got back an integer from this function rather than the native True and False values. I also want to emphasize that this change is specifically targeting Python 3.6, not 2.7, 3.4, or 3.5. Making this small change in a not-yet-released version should be safe, but, of course, that is my outsider opinion. You absolutely have a valid point if I were asking that we change a released version and it is not something I take lightly.

Lastly, this is the kind of change I would hope I could make easily as a first time contributor to the project because of it being more of an aesthetic one. After all, isn't the whole point of making changes to a project about making it cleaner, modern, and clearer?

Thanks again. I really appreciate the feedback and discussion. P.S. If there is support to introduce this change, I will certainly update the tests and docs as pointed out.
msg272406 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2016-08-11 07:02
Sorry Joseph, I concur that with Ned that we're best off leaving this alone (risk of breaking doctests with the changing repr, risk of fooling an overly specific type test, etc).  The code is not incorrect and does correspond with its docstring which claims 1 for true and 2 for false.

The sentiment is appreciated, but in general, the time to get APIs perfect is when it is initially released.  Afterwards, we really only want to make implementation improvements or API extensions until the API is flat-out broken or has major usability problems (even then, it always causes consternation to change the API).
History
Date User Action Args
2020-06-22 12:20:14Nathaniel Manistasetnosy: + Nathaniel Manista
2016-08-11 07:14:25SilentGhostsetresolution: not a bug -> wont fix
stage: patch review -> resolved
2016-08-11 07:02:44rhettingersetstatus: open -> closed

nosy: + rhettinger
messages: + msg272406

resolution: not a bug
2016-08-10 13:27:38Joseph Banesetmessages: + msg272331
2016-08-10 00:56:47ned.deilysetnosy: + ned.deily
messages: + msg272284
2016-08-09 23:55:17berker.peksagsetnosy: + berker.peksag

messages: + msg272282
stage: commit review -> patch review
2016-08-09 20:37:43SilentGhostsetmessages: + msg272267
stage: commit review
2016-08-09 20:12:06Joseph Banesetfiles: + patch.diff

messages: + msg272266
2016-08-09 19:57:31SilentGhostsettype: enhancement -> behavior

messages: + msg272265
nosy: + SilentGhost
2016-08-09 19:40:57r.david.murraysetversions: + Python 3.6, - Python 2.7, Python 3.5
2016-08-09 19:40:33r.david.murraysettitle: strtobool returns 0 and 1 rather than False and True -> distutils strtobool returns 0 and 1 rather than False and True
2016-08-09 19:35:52Joseph Banecreate