gh-100450: Add sqlite.Row.__contains__ method

[sobolevn]

I went ahead and added it :)

CC @erlend-aasland and @vsajip

• Issue: sqlite3.Row behaves like a dictionary but doesn't handle "in" as one might expect #100450

[erlend-aasland]

I left some nitpicks. Also, please add tests.

[erlend-aasland]

Please use intermingled declarations :)

[erlend-aasland]

There's a lot happening on this line. I'd prefer it if you extracted these items before calling PyObject_RichCompareBool, for improved readability.

[bedevere-bot]

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

[pochmann]

@erlend-aasland Hmm, is this whole thing not wrong anyway? (See my comment in the issue thread.)

[sobolevn]

@erlend-aasland

Also, please add tests.

I've added two doctests, aren't they enough?
If not, I don't know what is the best place for these tests. I can make new file for Row tests 🤔

[erlend-aasland]

I've added two doctests, aren't they enough? If not, I don't know what is the best place for these tests.

The doctests are there to make sure the examples in the docs actually work; it is not the place for unit tests. The unit tests (and regression tests, functional tests, etc.) all live in Lib/test/test_sqlite3. Their purpose is to test the _sqlite extension module (and the sqlite3 module) in all possible ways; we aim for as high code coverage as possible.

If I want to check the code coverage of the _sqlite C code, I can do so easily with a couple of compiler/linker flags, running ./python.exe -m test test_sqlite3, and generate a report.

Also, I'm not sure the buildbots run doctests; IIRC, they only run the test suite, but I might be wrong about this.

[sobolevn]

Sure, @erlend-aasland, I've added a unit test :)

[erlend-aasland]

Sure, @erlend-aasland, I've added a unit test :)

Thanks! BTW; I haven't decided if this is a good thing to add, or not. But lets keep that conversation on the issue. I'm going to mark this with do-not-merge until the discussion has landed.

[vsajip]

I haven't decided if this is a good thing to add

Should we open this to wider discussion on Discourse/Core development?

[erlend-aasland]

Yeah, IMO it is worth it trying to gather more feedback on Discourse.