bpo-44640: Improve punctuation in isinstance() error message #27144

wyz23x2 · 2021-07-14T17:51:14Z

Unify the punctuation marks of isinstance() and issubclass() error messages.

https://bugs.python.org/issue44640

wyz23x2 · 2021-07-14T18:06:23Z

Does this need news? IMO it's a very minor change, but it involves frequently-used builtins.

jdevries3133 · 2021-07-14T18:11:17Z

The word "union" is actually an exception to the "a/an" rule. Here is more detail:

https://english.stackexchange.com/questions/266309/why-is-union-an-exception-to-the-a-an-rule

wyz23x2 · 2021-07-14T18:16:17Z

OK :) English isn't my first language.

jdevries3133 · 2021-07-14T18:59:29Z

@wyz23x2 It's all good! English is very fickle and the fact that "union" is an exception to the a/an rule is a pretty obscure one.

To be clear, I don't think that the punctuation removal is necessary either; the original message is a grammatically correct full sentence. In its current state, you left behind the first comma, and removed all the following punctuation, which is not correct.

Unfortunately, as far as I can tell, there are no changes needed here; but feel free to let me know if you think I'm missing something.

wyz23x2 · 2021-07-15T02:04:13Z

I agree it's somehow a "micro" change and maybe not needed.

..., and removed all the following punctuation, which is not correct.

I just thought it's strange because all others aren't really full sentences since they have no periods:

>>> import math, sys
>>> int([])
Traceback (most recent call last):
  ...
TypeError: int() argument must be a string, a bytes-like object or a real number, not 'list'
>>> math.sqrt(-1)
Traceback (most recent call last):
  ...
ValueError: math domain error
>>> [3.14][sys.maxsize+100]
Traceback (most recent call last):
  ...
IndexError: cannot fit 'int' into an index-sized integer
>>> import mathh
Traceback (most recent call last):
  ...
ModuleNotFoundError: No module named 'mathh'

etc.
Changing all of them would require a quite big rewrite of the test suite (and I think there's lots of them in the stdlib!).

jdevries3133 · 2021-07-15T03:59:52Z

But then again, those other messages are simpler single-statement messages. Since this message is a three-part list, maybe the punctuation in this case provides some needed clarity?

In any case, you have to think about those chaotic individuals who have written code which asserts against the content of the error message here. Python is big enough that it probably has happened, and this change will break that person's (albeit questionable) code.

Objects/abstract.c

Fidget-Spinner · 2021-07-15T09:54:31Z

In any case, you have to think about those chaotic individuals who have written code which asserts against the content of the error message here. Python is big enough that it probably has happened, and this change will break that person's (albeit questionable) code.

It's unlikely for there to be much code reliant on that. Union support is 3.10 onwards only, and 3.10.0 isn't out yet (only the betas). So I don't think any code is going to break.

wyz23x2 · 2021-07-16T10:50:25Z

But then again, those other messages are simpler single-statement messages. Since this message is a three-part list, maybe the punctuation in this case provides some needed clarity?

While, the int() error message is also similar format, but doesn't have a comma before or (but maybe because of , not blah after it), and doesn't end with a period. But changing that would be strange if float() and complex() remain original, and may also break tests.

wyz23x2 · 2021-07-16T10:54:20Z

P.S. I'm sure that it was noted before, but actually any object that implements __int__, __trunc__ or __index__ is supported, not just the ones specified in the int() message. Same for float and complex.

Fidget-Spinner

LGTM. I'll leave it up to the core dev merging this to decide whether we should backport this to 3.10 or not (personally, I don't think it will hurt, but this doesn't look like a something we usually backport either so I'm not too sure).

jacobtylerwalls · 2021-07-18T20:57:32Z

Without offering an opinion on whether this change is desirable, just offering that you could say "Add serial comma" rather than "Fix punctuation". Yes, a period is added also, but if this is change is worth merging, it would likely only be for the serial comma.

wyz23x2 · 2021-08-06T04:13:06Z

Ping.

miss-islington · 2021-09-17T23:10:20Z

Thanks @wyz23x2 for the PR, and @ambv for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10.
🐍🍒⛏🤖

bedevere-bot · 2021-09-17T23:10:32Z

GH-28436 is a backport of this pull request to the 3.10 branch.

…rror messages (pythonGH-27144) Co-authored-by: Łukasz Langa <[email protected]> (cherry picked from commit f481338) Co-authored-by: wyz23x2 <[email protected]>

…rror messages (GH-27144) (GH-28436) Co-authored-by: Łukasz Langa <[email protected]> (cherry picked from commit f481338) Co-authored-by: wyz23x2 <[email protected]> Co-authored-by: wyz23x2 <[email protected]>

…rror messages (pythonGH-27144) (pythonGH-28436) Co-authored-by: Łukasz Langa <[email protected]> (cherry picked from commit f481338) Co-authored-by: wyz23x2 <[email protected]> Co-authored-by: wyz23x2 <[email protected]>

bedevere-bot · 2021-09-20T09:15:08Z

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot x86-64 macOS 3.10 has failed when building commit 9c23a1e.

What do you need to do:

Don't panic.
Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/681/builds/396) and take a look at the build logs.
Check if the failure is related to this commit (9c23a1e) or if it is a false positive.
If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/all/#builders/681/builds/396

Failed tests:

test_importlib

Failed subtests:

test_multiprocessing_pool_circular_import - test.test_importlib.test_threaded_import.ThreadedImportTests

Summary of the results of the build (if available):

== Tests result: FAILURE then FAILURE ==

408 tests OK.

10 slowest tests:

test_concurrent_futures: 3 min 46 sec
test_multiprocessing_spawn: 3 min 32 sec
test_unparse: 3 min 10 sec
test_tokenize: 2 min 13 sec
test_multiprocessing_forkserver: 2 min 9 sec
test_lib2to3: 2 min 4 sec
test_unicodedata: 1 min 49 sec
test_capi: 1 min 46 sec
test_asyncio: 1 min 42 sec
test_io: 1 min 3 sec

1 test failed:
test_importlib

18 tests skipped:
test_devpoll test_epoll test_gdb test_ioctl test_msilib
test_multiprocessing_fork test_ossaudiodev test_smtpnet test_spwd
test_ssl test_startfile test_tix test_tk test_ttk_guionly
test_winconsoleio test_winreg test_winsound test_zipfile64

1 re-run test:
test_importlib

Total duration: 32 min 36 sec

Click to see traceback logs

Traceback (most recent call last):
  File "/Users/buildbot/buildarea/3.10.billenstein-macos/build/Lib/test/test_importlib/partial/pool_in_threads.py", line 9, in t
    with multiprocessing.Pool(1):
  File "/Users/buildbot/buildarea/3.10.billenstein-macos/build/Lib/multiprocessing/context.py", line 119, in Pool
    return Pool(processes, initializer, initargs, maxtasksperchild,
  File "/Users/buildbot/buildarea/3.10.billenstein-macos/build/Lib/multiprocessing/pool.py", line 212, in __init__
    self._repopulate_pool()
  File "/Users/buildbot/buildarea/3.10.billenstein-macos/build/Lib/multiprocessing/pool.py", line 303, in _repopulate_pool
    return self._repopulate_pool_static(self._ctx, self.Process,
  File "/Users/buildbot/buildarea/3.10.billenstein-macos/build/Lib/multiprocessing/pool.py", line 326, in _repopulate_pool_static
    w.start()
  File "/Users/buildbot/buildarea/3.10.billenstein-macos/build/Lib/multiprocessing/process.py", line 121, in start
    self._popen = self._Popen(self)
  File "/Users/buildbot/buildarea/3.10.billenstein-macos/build/Lib/multiprocessing/context.py", line 284, in _Popen
    return Popen(process_obj)
  File "/Users/buildbot/buildarea/3.10.billenstein-macos/build/Lib/multiprocessing/popen_spawn_posix.py", line 32, in __init__
    super().__init__(process_obj)
  File "/Users/buildbot/buildarea/3.10.billenstein-macos/build/Lib/multiprocessing/popen_fork.py", line 19, in __init__
    self._launch(process_obj)
  File "/Users/buildbot/buildarea/3.10.billenstein-macos/build/Lib/multiprocessing/popen_spawn_posix.py", line 53, in _launch
    parent_r, child_w = os.pipe()
OSError: [Errno 24] Too many open files
/Users/buildbot/buildarea/3.10.billenstein-macos/build/Lib/multiprocessing/resource_tracker.py:224: UserWarning: resource_tracker: There appear to be 102 leaked semaphore objects to clean up at shutdown
  warnings.warn('resource_tracker: There appear to be %d '
---


Traceback (most recent call last):
  File "/Users/buildbot/buildarea/3.10.billenstein-macos/build/Lib/test/test_importlib/test_threaded_import.py", line 258, in test_multiprocessing_pool_circular_import
    script_helper.assert_python_ok(fn)
  File "/Users/buildbot/buildarea/3.10.billenstein-macos/build/Lib/test/support/script_helper.py", line 160, in assert_python_ok
    return _assert_python(True, *args, **env_vars)
  File "/Users/buildbot/buildarea/3.10.billenstein-macos/build/Lib/test/support/script_helper.py", line 145, in _assert_python
    res.fail(cmd_line)
  File "/Users/buildbot/buildarea/3.10.billenstein-macos/build/Lib/test/support/script_helper.py", line 72, in fail
    raise AssertionError("Process return code is %d\n"
AssertionError: Process return code is 1
command line: ['/Users/buildbot/buildarea/3.10.billenstein-macos/build/python.exe', '-X', 'faulthandler', '-I', '/Users/buildbot/buildarea/3.10.billenstein-macos/build/Lib/test/test_importlib/partial/pool_in_threads.py']

…rror messages (pythonGH-27144) Co-authored-by: Łukasz Langa <[email protected]>

bpo-44640: Fix typos of isinstance() and issubclass() error messages

b9eb950

bedevere-bot added the awaiting review label Jul 14, 2021

the-knights-who-say-ni added the CLA signed label Jul 14, 2021

Fidget-Spinner added the skip news label Jul 14, 2021

bpo-44640: Unify punctuation in builtin error messages

3b027a5

ambv reviewed Jul 15, 2021

View reviewed changes

Objects/abstract.c Outdated Show resolved Hide resolved

bpo-44640: Add comma and period to isinstance() error message.

88a57aa

wyz23x2 changed the title ~~bpo-44640: Fix typos in isinstance() and issubclass() error messages~~ bpo-44640: Fix punctuation in isinstance() error message Jul 16, 2021

Fidget-Spinner approved these changes Jul 16, 2021

View reviewed changes

bedevere-bot added awaiting core review and removed awaiting review labels Jul 16, 2021

wyz23x2 changed the title ~~bpo-44640: Fix punctuation in isinstance() error message~~ bpo-44640: Improve punctuation in isinstance() error message Jul 19, 2021

ambv added 2 commits September 18, 2021 00:46

Merge remote-tracking branch 'upstream/main' into patch-4

bc67a9f

Drop the period, it wasn't there before 3.10

00f94c0

ambv added the needs backport to 3.10 only security fixes label Sep 17, 2021

ambv merged commit f481338 into python:main Sep 17, 2021

bedevere-bot removed the awaiting core review label Sep 17, 2021

bedevere-bot removed the needs backport to 3.10 only security fixes label Sep 17, 2021

niyas-sait pushed a commit to niyas-sait/cpython that referenced this pull request Sep 21, 2021

bpo-44640: Improve punctuation consistency in isinstance/issubclass e…

a03e2d5

…rror messages (pythonGH-27144) Co-authored-by: Łukasz Langa <[email protected]>

wyz23x2 deleted the patch-4 branch October 10, 2021 01:25

Uh oh!

bpo-44640: Improve punctuation in isinstance() error message #27144

bpo-44640: Improve punctuation in isinstance() error message #27144

Uh oh!

Conversation

wyz23x2 commented Jul 14, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wyz23x2 commented Jul 14, 2021

Uh oh!

jdevries3133 commented Jul 14, 2021

Uh oh!

wyz23x2 commented Jul 14, 2021

Uh oh!

jdevries3133 commented Jul 14, 2021

Uh oh!

wyz23x2 commented Jul 15, 2021

Uh oh!

jdevries3133 commented Jul 15, 2021

Uh oh!

Uh oh!

Fidget-Spinner commented Jul 15, 2021

Uh oh!

wyz23x2 commented Jul 16, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wyz23x2 commented Jul 16, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Fidget-Spinner left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jacobtylerwalls commented Jul 18, 2021

Uh oh!

wyz23x2 commented Aug 6, 2021

Uh oh!

miss-islington commented Sep 17, 2021

Uh oh!

bedevere-bot commented Sep 17, 2021

Uh oh!

bedevere-bot commented Sep 20, 2021

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Uh oh!

Uh oh!

wyz23x2 commented Jul 14, 2021 •

edited

Loading

wyz23x2 commented Jul 16, 2021 •

edited

Loading

wyz23x2 commented Jul 16, 2021 •

edited

Loading

Fidget-Spinner left a comment •

edited

Loading