classification
Title: Better repr for threading.Lock()
Type: enhancement Stage: resolved
Components: Library (Lib) Versions: Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: rhettinger Nosy List: berker.peksag, pitrou, python-dev, rhettinger, serhiy.storchaka
Priority: normal Keywords: easy, patch

Created on 2014-04-03 01:19 by rhettinger, last changed 2014-05-26 05:21 by berker.peksag. This issue is now closed.

Files
File name Uploaded Description Edit
issue21137.diff berker.peksag, 2014-04-03 02:14 review
issue21137_v2.diff berker.peksag, 2014-04-30 18:32 review
issue21137_v3.diff berker.peksag, 2014-05-23 03:39 review
Messages (10)
msg215413 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2014-04-03 01:19
It is really nice to have the open/closed status in the __repr__ of file objects: 

  <open file 'data.txt', mode 'r' at 0x102c8ec90>
  <closed file 'data.txt', mode 'r' at 0x102c8ec90>

I would like to have the same for locks:

  <unlocked thread.lock object at 0x1002b1330>
  <locked thread.lock object at 0x1002b1330>

This would be nice in logs and tracebacks for example.
msg215416 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2014-04-03 02:14
Here's a patch with a test. Example repr of threading.Lock():

    $ ./python -c "import threading; print(threading.Lock())"
    <unlocked _thread.lock object>
msg215436 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-04-03 10:12
The repr of threading._RLock contains owner and count, but not lock/unlock status. The repr of locks from _dummy_thread also should contain lock/unlock status. And it would be nice to have the open/closed status in the repr of other synchronization primitives: Condition, Semaphore, etc. 

As for tests, I think assertRegex() will produce more useful error report.
msg215437 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-04-03 10:14
And of course we should keep "at 0x..." part, because it is the way to distinguish different lock objects.
msg217440 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2014-04-28 23:01
> The repr of threading._RLock contains owner and count, but not
> lock/unlock status. The repr of locks from _dummy_thread also should
> contain lock/unlock status. And it would be nice to have the
> open/closed status in the repr of other synchronization primitives:
> Condition, Semaphore, etc. 

I think it's ok to stay focussed and restrict this issue to threading.Lock (and perhaps RLock).

Berker, do you want to provide an updated patch?
msg217637 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2014-04-30 18:32
Here's an updated patch. Thanks for the reviews.

> And of course we should keep "at 0x..." part, because it is the way to distinguish different lock objects.

Done.

    $ ./python -c "import threading; l = threading.Lock(); print(l)"
    <unlocked _thread.lock object at 0x7f0a19e7b1f8>


> The repr of threading._RLock contains owner and count, but not lock/unlock status.

Done.

    $ ./python -c "import threading; rl = threading.RLock(); rl.acquire(); print(rl)"
    <locked _thread.RLock object owner=139769600231168 count=1>


>  The repr of locks from _dummy_thread also should contain lock/unlock status.

Done.

    $ ./python -c "import dummy_threading as threading; l = threading.Lock(); print(l)"
    <unlocked _dummy_thread.LockType object at 0x7fb334245400>

    $ ./python -c "import dummy_threading as threading; l = threading.RLock(); print(l)"
    <unlocked threading._RLock object owner=None count=0 at 0x7f524d0138e0>


> As for tests, I think assertRegex() will produce more useful error report.

Done.
msg218940 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2014-05-23 03:02
The patch is just about ready to go but needs to address some failing tests:

======================================================================
FAIL: test_locked_repr (test.test_importlib.test_locks.Frozen_ModuleLockAsRLockTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/raymond/cpython/Lib/test/lock_tests.py", line 91, in test_locked_repr
    self.assertRegex(repr(lock), "<locked .* object (.*)?at .*>")
AssertionError: Regex didn't match: '<locked .* object (.*)?at .*>' not found in "_ModuleLock('some_lock') at 4438138048"

======================================================================
FAIL: test_repr (test.test_importlib.test_locks.Frozen_ModuleLockAsRLockTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/raymond/cpython/Lib/test/lock_tests.py", line 85, in test_repr
    self.assertRegex(repr(lock), "<unlocked .* object (.*)?at .*>")
AssertionError: Regex didn't match: '<unlocked .* object (.*)?at .*>' not found in "_ModuleLock('some_lock') at 4439151784"

======================================================================
FAIL: test_locked_repr (test.test_importlib.test_locks.Source_ModuleLockAsRLockTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/raymond/cpython/Lib/test/lock_tests.py", line 91, in test_locked_repr
    self.assertRegex(repr(lock), "<locked .* object (.*)?at .*>")
AssertionError: Regex didn't match: '<locked .* object (.*)?at .*>' not found in "_ModuleLock('some_lock') at 4438192312"

======================================================================
FAIL: test_repr (test.test_importlib.test_locks.Source_ModuleLockAsRLockTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/raymond/cpython/Lib/test/lock_tests.py", line 85, in test_repr
    self.assertRegex(repr(lock), "<unlocked .* object (.*)?at .*>")
AssertionError: Regex didn't match: '<unlocked .* object (.*)?at .*>' not found in "_ModuleLock('some_lock') at 4438192480"

----------------------------------------------------------------------
Ran 970 tests in 0.549s

FAILED (failures=4, skipped=7, expected failures=1)
test test_importlib failed
1 test failed:
    test_importlib
msg218942 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2014-05-23 03:39
Here is a new patch. I've disabled test_repr and test_locked_repr tests in Lib/test/test_importlib/test_locks.py.
msg219116 - (view) Author: Roundup Robot (python-dev) Date: 2014-05-26 01:22
New changeset 7574f94b1068 by Raymond Hettinger in branch 'default':
Issue 21137:  Better repr for threading.Lock()
http://hg.python.org/cpython/rev/7574f94b1068
msg219117 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2014-05-26 01:23
Thanks Berker.
History
Date User Action Args
2014-05-26 05:21:13berker.peksagsetstage: patch review -> resolved
2014-05-26 01:23:20rhettingersetstatus: open -> closed
resolution: fixed
messages: + msg219117
2014-05-26 01:22:44python-devsetnosy: + python-dev
messages: + msg219116
2014-05-23 03:39:33berker.peksagsetfiles: + issue21137_v3.diff

messages: + msg218942
2014-05-23 03:02:10rhettingersetmessages: + msg218940
2014-05-23 02:48:43rhettingersetassignee: rhettinger
2014-04-30 18:32:10berker.peksagsetfiles: + issue21137_v2.diff

messages: + msg217637
2014-04-28 23:01:56pitrousetnosy: + pitrou
messages: + msg217440
2014-04-03 10:14:50serhiy.storchakasetmessages: + msg215437
2014-04-03 10:12:37serhiy.storchakasetnosy: + serhiy.storchaka

messages: + msg215436
stage: patch review
2014-04-03 02:14:28berker.peksagsetfiles: + issue21137.diff

nosy: + berker.peksag
messages: + msg215416

keywords: + patch
2014-04-03 01:19:02rhettingercreate