bpo-41833: threading.Thread now uses the target name #22357

vstinner · 2020-09-22T11:55:40Z

https://bugs.python.org/issue41833

vstinner · 2020-09-22T11:56:02Z

cc @pablogsal @methane @pitrou @serhiy-storchaka

pablogsal

LGTM

I left a minor nit

Doc/library/threading.rst

vstinner · 2020-09-22T15:07:42Z

@serhiy-storchaka raised some concerns in https://bugs.python.org/issue41833

vstinner · 2020-09-23T16:25:23Z

I rewrote my PR to generate names like "Thread-3 (func)", rather than just "func". So if two target functions have the same name, or if two threads use the same target, it remains possible to distinguish the two threads.

cc @serhiy-storchaka

serhiy-storchaka · 2020-09-23T16:29:05Z

Lib/threading.py

@@ -800,8 +799,20 @@ class is implemented.
        assert group is None, "group argument must be None for now"
        if kwargs is None:
            kwargs = {}
+        if name:
+            name = str(name)
+        if not name:


Suggested change

if not name:

else:

I would like to avoid empty thread name, and so I check that the result of str(name) is non-empty. Do you think that it's not worth it?

The current code allows to set an empty name, and with else the code is simpler. But it does not matter.

I agree with Serhiy's suggestion, if someone uses an empty string it was probably intentional. I don't think it's worth trying to catch it.

serhiy-storchaka · 2020-09-23T16:32:18Z

Lib/threading.py

+                    name_template = f"{name_template} ({target_name})"
+                except AttributeError:
+                    pass
+            name = _newname(name_template)


It will fail if target.__name__ contains % (please add a test for such case, and include also { just for the case).

It would be safer to write:

name = _newname() if target is not None: ... name = f"{name} ({target_name})"

I wrote the code differently to avoid formatting errors.

vstinner · 2020-09-23T16:57:38Z

@ammaraskar @pablogsal: Would you mind to review the updated PR?

serhiy-storchaka · 2020-09-23T16:59:14Z

Lib/threading.py

@@ -800,8 +799,20 @@ class is implemented.
        assert group is None, "group argument must be None for now"
        if kwargs is None:
            kwargs = {}
+        if name:
+            name = str(name)
+        if not name:


The current code allows to set an empty name, and with else the code is simpler. But it does not matter.

serhiy-storchaka · 2020-09-23T17:02:09Z

I am wondering in what case you would specify a non-string (and non-None) name.

ammaraskar

I am wondering in what case you would specify a non-string (and non-None) name.

Probably a numeric id? Still, it's probably better to maintain the old behavior.

LGTM with one small question around the test.

Lib/test/test_threading.py

ammaraskar · 2020-09-23T17:13:14Z

Lib/threading.py

@@ -800,8 +799,20 @@ class is implemented.
        assert group is None, "group argument must be None for now"
        if kwargs is None:
            kwargs = {}
+        if name:
+            name = str(name)
+        if not name:


I agree with Serhiy's suggestion, if someone uses an empty string it was probably intentional. I don't think it's worth trying to catch it.

vstinner · 2020-09-23T19:38:29Z

Ok, I changed the code to use else:.

I agree with Serhiy's suggestion, if someone uses an empty string it was probably intentional. I don't think it's worth trying to catch it.

In Python 3.9, threading.Thread(name='').name returns 'Thread-1'. My PR didn't change that and still doesn't change that.

I tried to change the behavior for such special case:

import threading

class Name:
    def __nonzero__(self):
        return True
    def __str__(self):
        return ''

name = threading.Thread(name=Name()).name
print(repr(name))

name is an empty string before and after my change (with my latest commit).

vstinner · 2020-09-23T21:22:23Z

Thanks for reviews and help to decide what is the correct way to name threads ;-) I merged my PR.

If someone wants to adjust the behavior for bool(name) being false, or empty name string, you can write a new PR ;-)

* origin/master: (27 commits) bpo-41428: Fix compiler warnings in unionobject.c (pythonGH-22388) bpo-41654: Fix compiler warning in MemoryError_dealloc() (pythonGH-22387) bpo-41833: threading.Thread now uses the target name (pythonGH-22357) bpo-30155: Add macros to get tzinfo from datetime instances (pythonGH-21633) bpo-33822: Update IDLE section of What's New 3.8 (pythonGH-22383) bpo-41844: Add IDLE section to What's New 3.9 (GN-22382) bpo-41841: Prepare IDLE News for 3.10 (pythonGH-22379) bpo-37779 : Add information about the overriding behavior of ConfigParser.read (pythonGH-15177) bpo-40170: Use inline _PyType_HasFeature() function (pythonGH-22375) bpo-40941: Fix stackdepth compiler warnings (pythonGH-22377) bpo-40941: Fix fold_tuple_on_constants() compiler warnings (pythonGH-22378) bpo-40521: Fix PyUnicode_InternInPlace() (pythonGH-22376) bpo-41834: Remove _Py_CheckRecursionLimit variable (pythonGH-22359) bpo-1635741, unicodedata: add ucd_type parameter to UCD_Check() macro (pythonGH-22328) bpo-1635741: Port _lsprof extension to multi-phase init (PEP 489) (pythonGH-22220) bpo-41513: Improve order of adding fractional values. Improve variable names. (pythonGH-22368) bpo-41816: `StrEnum.__str__` is `str.__str__` (pythonGH-22362) bpo-35764: Rewrite the IDLE Calltips doc section (pythonGH-22363) bpo-41810: Reintroduce `types.EllipsisType`, `.NoneType` & `.NotImplementedType` (pythonGH-22336) bpo-41602: raise SIGINT exit code on KeyboardInterrupt from pymain_run_module (python#21956) ...

* origin/master: (113 commits) bpo-41773: Raise exception for non-finite weights in random.choices(). (pythonGH-22441) bpo-41873: Add vectorcall for float() (pythonGH-22432) bpo-41861: Convert _sqlite3 PrepareProtocolType to heap type (pythonGH-22428) bpo-41842: Add codecs.unregister() function (pythonGH-22360) bpo-41875: Use __builtin_unreachable when possible (pythonGH-22433) bpo-40105: ZipFile truncate in append mode with shorter comment (pythonGH-19337) bpo-41870: Use PEP 590 vectorcall to speed up bool() (pythonGH-22427) [doc] Leverage the fact that the actual types can now be indexed for typing (pythonGH-22340) bpo-41861: Convert _sqlite3 cache and node static types to heap types (pythonGH-22417) bpo-41858: Clarify line in optparse doc (pythonGH-22407) Revert "Fix all Python Cookbook links (python#22205)" (pythonGH-22424) bpo-1635741: Port _bisect module to multi-phase init (pythonGH-22415) bpo-41428: Fix compiler warning in unionobject.c (pythonGH-22416) Fix logging error message (pythonGH-22410) bpo-39934: Account for control blocks in 'except' in compiler. (pythonGH-22395) bpo-41775: Make 'IDLE Shell' the shell title (python#22399) bpo-41428: Fix compiler warnings in unionobject.c (pythonGH-22388) bpo-41654: Fix compiler warning in MemoryError_dealloc() (pythonGH-22387) bpo-41833: threading.Thread now uses the target name (pythonGH-22357) bpo-30155: Add macros to get tzinfo from datetime instances (pythonGH-21633) ...

the-knights-who-say-ni added the CLA signed label Sep 22, 2020

bedevere-bot added the awaiting core review label Sep 22, 2020

pablogsal approved these changes Sep 22, 2020

View reviewed changes

Doc/library/threading.rst Outdated Show resolved Hide resolved

bedevere-bot added awaiting merge and removed awaiting core review labels Sep 22, 2020

pablogsal reviewed Sep 22, 2020

View reviewed changes

Doc/library/threading.rst Outdated Show resolved Hide resolved

bpo-41833: threading.Thread now uses the target name

8b4397d

serhiy-storchaka reviewed Sep 23, 2020

View reviewed changes

Avoid formatting errors

51b15fb

serhiy-storchaka approved these changes Sep 23, 2020

View reviewed changes

ammaraskar approved these changes Sep 23, 2020

View reviewed changes

Allow to set the name to an empty string

859b4c2

vstinner merged commit 98c16c9 into python:master Sep 23, 2020

bedevere-bot removed the awaiting merge label Sep 23, 2020

vstinner deleted the thread_name branch September 23, 2020 21:21

xzy3 pushed a commit to xzy3/cpython that referenced this pull request Oct 18, 2020

bpo-41833: threading.Thread now uses the target name (pythonGH-22357)

ef5e98e

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bpo-41833: threading.Thread now uses the target name #22357

bpo-41833: threading.Thread now uses the target name #22357

vstinner commented Sep 22, 2020 •

edited by bedevere-bot

Loading

vstinner commented Sep 22, 2020

pablogsal left a comment

vstinner commented Sep 22, 2020

vstinner commented Sep 23, 2020

serhiy-storchaka Sep 23, 2020

vstinner Sep 23, 2020

serhiy-storchaka Sep 23, 2020

ammaraskar Sep 23, 2020

serhiy-storchaka Sep 23, 2020

vstinner Sep 23, 2020

vstinner commented Sep 23, 2020

serhiy-storchaka Sep 23, 2020

serhiy-storchaka commented Sep 23, 2020

ammaraskar left a comment

ammaraskar Sep 23, 2020

vstinner commented Sep 23, 2020

vstinner commented Sep 23, 2020

bpo-41833: threading.Thread now uses the target name #22357

bpo-41833: threading.Thread now uses the target name #22357

Conversation

vstinner commented Sep 22, 2020 • edited by bedevere-bot Loading

vstinner commented Sep 22, 2020

pablogsal left a comment

Choose a reason for hiding this comment

vstinner commented Sep 22, 2020

vstinner commented Sep 23, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vstinner commented Sep 23, 2020

Choose a reason for hiding this comment

serhiy-storchaka commented Sep 23, 2020

ammaraskar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vstinner commented Sep 23, 2020

vstinner commented Sep 23, 2020

vstinner commented Sep 22, 2020 •

edited by bedevere-bot

Loading