tobgu / pyrsistent

Persistent/Immutable/Functional data structures for Python
MIT License
2.03k stars 147 forks source link

PMap set or __contains__ unreliable in 0.19.1 #263

Closed mwchase closed 2 years ago

mwchase commented 2 years ago

I'm sorry I don't have a better thing to look for here, but the code that hits this is deep in the guts of an application that only I use, and I haven't yet worked out how to extract a reproduction.

Here's what I've got:

I have a frozen attrs class that contains a variety of pyrsistent data structures. Routine tests of the operation of this structure reveal that sometimes, a PMap unexpectedly reports that it does not contain a particular key. Whether it reports this, and which key it reports, varies at random. I believe both string keys, and other attrs classes have been keys that failed.

On 0.19.1 this behavior manifests inconsistently. On 0.18.1 this behavior does not manifest, so this represents a regression.

Looking over the history of the pmap file, I wonder if https://github.com/tobgu/pyrsistent/commit/0898ff7ac62c1a4d48c2046962b8425bc9b70fc5 may have broken some invariant.

I believe the maps in question contain no more than a few dozen keys in the cases that hit this.

For now, I can continue developing this application using 0.18.X, but I wanted to make sure this doesn't fall through the cracks. I'll see about getting a minimal reproduction together soon.

tobgu commented 2 years ago

Thanks for reporting this! This was a nasty regression indeed. Fixed in 0.19.2, please verify that it works for you and re-open this issue if you still have problems.

mwchase commented 2 years ago

It looks like it's working now, thank you!

I have confirmed that pip freeze output in the relevant virtual environment includes pyrsistent==0.19.2, and one of the tests that was failing somewhere around half the time is now passing every time.