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

Discard transformation adds nodes #245

Closed
mut3 opened this issue Mar 15, 2022 · 3 comments
Closed

Discard transformation adds nodes #245

mut3 opened this issue Mar 15, 2022 · 3 comments

Comments

@mut3
Copy link

mut3 commented Mar 15, 2022

30f1381 changed the way transforms work so that they add intervening nodes from the matcher which didn't previously exist.

This addressed the case in #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
Copy link
Owner

tobgu commented Mar 15, 2022

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

@tobgu tobgu closed this as completed in 07d60a5 Oct 22, 2023
@tobgu
Copy link
Owner

tobgu commented Oct 22, 2023

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

@mut3
Copy link
Author

mut3 commented Oct 27, 2023

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

No problem, thank you for the fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants