glom icon indicating copy to clipboard operation
glom copied to clipboard

"Move" target mutation

Open sobolevn opened this issue 6 years ago • 4 comments

Sometimes we need to move data from key to key.

def format_data(webhook_data):
     return glom(webhook_data, (
            Assign('object_attributes.project', Spec('project')),
        ))

And then using it:

webhook_data = {'project': {'uid': 1}, 'object_attributes': {'uid': 2}}
modified = format_data(webhook_data)
# => {'project': {'uid': 1}, 'object_attributes': {'uid': 2, 'project': {'uid': 1}}}

It works, but this way we tend to use more memory that we possibly can. And storing extra 'project' key is not required. It maybe way too memory consuming when dictionaries are big. And there are many inputs.

So, that why I suggest to add Move class that will handle that.

With Move it will work like so:

def format_data(webhook_data):
     return glom(webhook_data, (
            Move('object_attributes.project', Spec('project')),
        ))

webhook_data = {'project': {'uid': 1}, 'object_attributes': {'uid': 2}}
modified = format_data(webhook_data)
# => {'object_attributes': {'uid': 2, 'project': {'uid': 1}}}

If you like the idea - I would be happy to work on it.

sobolevn avatar Feb 07 '19 13:02 sobolevn

Hey @sobolevn! Excellent to see you're getting into the nitty-gritty of glom! I have a couple of thoughts:

  1. I can see clearly how Move() would work on dicts and other mappings. But because glom can access attributes on lists and objects, I'm not sure how a value might get moved out in those cases. This got me thinking about the second thought:

  2. If the desired behavior is to pop a key and assign it to another location, maybe we can do that:

webhook_data = {'other_attributes': {'project': 'sweet_project', 'other_id': "123"}}
spec = T['other_attributes'].pop('project')
glom(webhook_data, Assign('project', Spec(spec)))
# {'project': 'sweet_project', 'other_attributes': {'other_id': '123'}}

Here we're using T, and it'll work with any object supporting .pop(key) (most mappings).


Overall I'm very excited to see this usage, and if you feel like there are straightforward answers to what Move() would do with non-mapping objects, I'd be happy to hear them. Otherwise, I'm more inclined to document this pattern in glom's snippets docs. :)

mahmoud avatar Feb 08 '19 02:02 mahmoud

@mahmoud hm, I have not considered lists and objects.

So, there are multiple cases how to handle them.

Lists

Exceptions

glom([1, 2, 3], Move('1', Spec('0')))
# => TypeError('Lists can not be moved, it does not make sence')

Insert

glom([1, 2, 3], Move('1', Spec('0')))  # the same as calling: pop(old_index) and insert(new_index)
# => [2, 1, 3]

Objects

With objects we can face different corner cases: __slots__, custom __getattr__ / __setattr__, properties, descriptors, etc.

But, I belive that it is the user's responsibility to think about all this corner cases and use glom where appropriate.

So, usage example:

@attr.s(auto_attribs=True)
class OldFormat(object):
     field_name: str

glom(OldFormat('text'), Move('field_name', Spec('property_name')))
# => Will work

And with

@attr.s(auto_attribs=True, slots=True, frozen=True)
class OldFormat(object):
     field_name: str

glom(OldFormat('text'), Move('field_name', Spec('property_name')))
# => FrozenException(...)

What do you think about it @mahmoud ?

sobolevn avatar Feb 08 '19 09:02 sobolevn

Hey again @sobolevn! Thanks for your patience on this, I assure you it's for good cause. I've been working on docs on how to create an idiomatic glom extension (custom specifier type) for exactly this purpose, and I think they're finally readable: https://glom.readthedocs.io/en/latest/extensions.html

I think it'll be easier to discuss semantics once we've got some actual code to comment on, provided it follows the patterns set forth in that doc. :)

Another thought I had here was around transactionality. If we set the new value, but fail to remove the old one (due to a custom delitem or something), would you want to unset the new value, too?

mahmoud avatar Feb 20 '19 17:02 mahmoud

Thanks @mahmoud!

I am pretty sure that leaving something in a broken state is not a good idea. So, this operations must be transactional. I guess, this will bring an additional overhead to the system. Maybe we can do the same default as SQL databases? If Move(..., transactional=True) is specified, then everything will be cleaned up in case of an exception.

sobolevn avatar Feb 20 '19 18:02 sobolevn