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

 kwargs={}
+ifname:
+name=str(name)
+ifnotname:


Suggested change
ifnotname:
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})"
+exceptAttributeError:
+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() iftargetisnotNone: ... 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

 kwargs={}
+ifname:
+name=str(name)
+ifnotname:


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

 kwargs={}
+ifname:
+name=str(name)
+ifnotname:


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 resolvedHide 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 resolvedHide 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:masterSep 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

Uh oh!

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

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

Uh oh!

Conversation

vstinner commented Sep 22, 2020• edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vstinner commented Sep 22, 2020

Uh oh!

pablogsal left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

vstinner commented Sep 22, 2020

Uh oh!

vstinner commented Sep 23, 2020

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!

vstinner commented Sep 23, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

serhiy-storchaka commented Sep 23, 2020

Uh oh!

ammaraskar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vstinner commented Sep 23, 2020

Uh oh!

vstinner commented Sep 23, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

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