Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better repr for threading.Lock() #65336

Closed
rhettinger opened this issue Apr 3, 2014 · 10 comments
Closed

Better repr for threading.Lock() #65336

rhettinger opened this issue Apr 3, 2014 · 10 comments
Assignees
Labels
easy stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@rhettinger
Copy link
Contributor

BPO 21137
Nosy @rhettinger, @pitrou, @berkerpeksag, @serhiy-storchaka
Files
  • issue21137.diff
  • issue21137_v2.diff
  • issue21137_v3.diff
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/rhettinger'
    closed_at = <Date 2014-05-26.01:23:20.591>
    created_at = <Date 2014-04-03.01:19:02.742>
    labels = ['easy', 'type-feature', 'library']
    title = 'Better repr for threading.Lock()'
    updated_at = <Date 2014-05-26.05:21:13.380>
    user = 'https://github.com/rhettinger'

    bugs.python.org fields:

    activity = <Date 2014-05-26.05:21:13.380>
    actor = 'berker.peksag'
    assignee = 'rhettinger'
    closed = True
    closed_date = <Date 2014-05-26.01:23:20.591>
    closer = 'rhettinger'
    components = ['Library (Lib)']
    creation = <Date 2014-04-03.01:19:02.742>
    creator = 'rhettinger'
    dependencies = []
    files = ['34710', '35118', '35321']
    hgrepos = []
    issue_num = 21137
    keywords = ['patch', 'easy']
    message_count = 10.0
    messages = ['215413', '215416', '215436', '215437', '217440', '217637', '218940', '218942', '219116', '219117']
    nosy_count = 5.0
    nosy_names = ['rhettinger', 'pitrou', 'python-dev', 'berker.peksag', 'serhiy.storchaka']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue21137'
    versions = ['Python 3.5']

    @rhettinger
    Copy link
    Contributor Author

    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.

    @rhettinger rhettinger added stdlib Python modules in the Lib dir easy type-feature A feature request or enhancement labels Apr 3, 2014
    @berkerpeksag
    Copy link
    Member

    Here's a patch with a test. Example repr of threading.Lock():

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

    @serhiy-storchaka
    Copy link
    Member

    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.

    @serhiy-storchaka
    Copy link
    Member

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

    @pitrou
    Copy link
    Member

    pitrou commented Apr 28, 2014

    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?

    @berkerpeksag
    Copy link
    Member

    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.

    @rhettinger rhettinger self-assigned this May 23, 2014
    @rhettinger
    Copy link
    Contributor Author

    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

    @berkerpeksag
    Copy link
    Member

    Here is a new patch. I've disabled test_repr and test_locked_repr tests in Lib/test/test_importlib/test_locks.py.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented May 26, 2014

    New changeset 7574f94b1068 by Raymond Hettinger in branch 'default':
    bpo-21137: Better repr for threading.Lock()
    http://hg.python.org/cpython/rev/7574f94b1068

    @rhettinger
    Copy link
    Contributor Author

    Thanks Berker.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    easy stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants