◐ Shell
clean mode source ↗

gh-91052: Add C API for watching dictionaries by carljm · Pull Request #31787 · python/cpython

@carljm

Notes on the PR for reviewers:

  • Since this API is intended for use by extensions, and _Py* APIs are internal-only, I added the API as public (Py*). But I think it should perhaps be CPython-specific, so I put it in Include/cpython/dictobject.h, not Include/dictobject.h. Not sure about any of this, though, so happy to change as requested.

itamaro

@MaxwellDupre

Do you have a test harness/or something I can use (without too much effort :)? I could just run the Python test suite but that may be missing the point.

@carljm

Do you have a test harness/or something I can use (without too much effort :)? I could just run the Python test suite but that may be missing the point.

Hi! I'm not sure what you are trying to achieve -- can you clarify your goals and how they relate to this PR?

@ghost

All commit authors signed the Contributor License Agreement.
CLA signed

@carljm carljm changed the title bpo-46896: Add C API for watching dictionaries gh-91052: Add C API for watching dictionaries

Oct 3, 2022

@carljm

@markshannon I've updated this PR to the design we discussed, would be glad for your review. I'll try to get some updated pyperformance numbers tomorrow.

@carljm

Fidget-Spinner

markshannon

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally looks good. A few comments.

markshannon

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few more comments, but looking good.

@markshannon

@sweeneyde

I think this may have introduced some refleaks into test_capi.

@carljm

Thanks for the heads up, I'll take a look.

iritkatriel

self.assertIs(unraisable.object, d)
self.assertEqual(str(unraisable.exc_value), "boom!")

def test_two_watchers(self):

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test is ref leaking on my PR: #98001

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah probably fixed in #98017 ?

@vstinner

@carljm

carljm added a commit to carljm/cpython that referenced this pull request

Oct 8, 2022

mpage pushed a commit to mpage/cpython that referenced this pull request

Oct 11, 2022