tobgu / pyrsistent

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

Discard transformation adds nodes #245

Closed mut3 closed 1 year ago

mut3 commented 2 years ago

https://github.com/tobgu/pyrsistent/commit/30f138142923669c9da327abe214986919d52a07 changed the way transforms work so that they add intervening nodes from the matcher which didn't previously exist.

This addressed the case in https://github.com/tobgu/pyrsistent/issues/154 and made

>>> freeze({}).transform(['foo','bar'],m())

Have the intuitive result:

pmap({'foo': pmap({'bar': pmap({})})})

Rather than

pmap({})

But I think this added an unintuitive side-effect to discard where it will add nodes.

in versions >=0.15.2:

>>> freeze({}).transform(['foo','bar'],discard)
pmap({'foo': pmap({})})

in versions <0.15.2:

>>> freeze({}).transform(['foo','bar'],discard)
pmap({})

I don't think discard should ever add anything to a PMap

This caused serious headache for me as we were using a transformation with discard to remove an empty map with a specific key if it existed, but when we bumped the version of pyrsistent we were using, that transformation started adding new empty maps one level up from the potential location of the empty map.

For example:

versions <0.15.2

>>> freeze({'baz':{'foo':{'bar':1}},'boo':{'far':2}}).transform([ny,'foo','bar'],discard)
pmap({'boo': pmap({'far': 2}), 'baz': pmap({'foo': pmap({})})}) # this is what I expect

versions >=0.15.2

>>> freeze({'baz':{'foo':{'bar':1}},'boo':{'far':2}}).transform([ny,'foo','bar'],discard)
pmap({'baz': pmap({'foo': pmap({})}), 'boo': pmap({'foo': pmap({}), 'far': 2})}) # added new PMap boo.foo
tobgu commented 2 years ago

Thanks for reporting this. Your comment about never adding nodes with discard seems very reasonable. I'll have a look!

tobgu commented 1 year ago

Sorry for the very late action on this. Fix will be released in v0.20.0.

mut3 commented 1 year ago

Sorry for the very late action on this. Fix will be released in v0.20.0.

No problem, thank you for the fix!