pyrsistent icon indicating copy to clipboard operation
pyrsistent copied to clipboard

`setdefault`, or some superior functionality as part of `transform()`

Open itamarst opened this issue 8 years ago • 5 comments
trafficstars

Consider the following code:

class ApplicationState(PClass):
    shared_resources = pset_field(Injectable)
    # ...

class ExtractedState(PClass):
    applications = pmap_field(str, ApplicationState)

def extract(tfstate):
    result = {}

    for mod in tfstate['modules']:
        for injectable in _extract_resources_from_module(mod):
            app_state = result.setdefault(injectable.app,
                                          {"shared_resources": set(),
                                           "service_resources": {}})
            app_state["shared_resources"].add(injectable)

   return ExtractedState.create(freeze({"applications": result}))

Constructing the pyrsistent objects is quite annoying, which is why I'm doing it with mutable objects, due to lack of setdefault or "create a default object in transform()". Ideally I'd do:

extracted_state = ExtractedState()
extracted_state = extracted_state.transform(
    ["applications", "newapp", "shared_resources"], lambda ps: ps.add(obj))

but since newapp doesn't exist it blows up...

itamarst avatar Feb 03 '17 19:02 itamarst

Seems cool, I can definitely see the use case.

At first I was thinking that the type information should be possible to use for auto building default structures. It seems to me that it would impose a lot of restrictions since the type information would have to be unambiguous (a type is specified, only one type is specified, the type is not abstract, etc.).

Furthermore I guess it may not always be the case that you want to do the exact same thing for every transformation involving a particular type. This means that the behaviour should probably not be tied to the type/class at all but instead be supplied as input to the transform operation. I guess this was what you were thinking as well since you mentioned setdefault.

What about making it possible to pass a function default(path) to transform that gets called with the current path (a list) as argument when a node specified in the transformation is missing? It would then return a node appropriate for the path (or ignore the path completely and just return a node if considered OK/safe).

extracted_state = extracted_state.transform(
    ["applications", "newapp", "shared_resources"], lambda ps: ps.add(obj), default=node_constructor_fn)

tobgu avatar Feb 07 '17 11:02 tobgu

That seems like it would require enough typing that it wouldn't save any time.

In practice with setdefault the vast majority of the time you just want an empty data structure ([], or default object with no arguments, etc.) So maybe just:

extracted_state = extracted_state.transform(
    ["applications", "newapp", "shared_resources"], lambda ps: ps.add(obj), add_defaults=True)

itamarst avatar Feb 07 '17 15:02 itamarst

(Or some other, better, argument name.)

itamarst avatar Feb 07 '17 15:02 itamarst

The only problem with that as I see it is that there are a lot of cases where such simplification would not be usable at all. For example when applying a transform to a data structure without type information (nested pvectors, pmaps and psets for example).

It's of course possible to combine the two suggestions and have a parameter (would calling it setdefault hint about the intent or just make it confusing?) that could either be True/False or a function as the one I suggested above. If set to True it would assume/require proper type information to be present and fail otherwise.

tobgu avatar Feb 07 '17 17:02 tobgu

Maybe instead of an option to transform() it could be a top-level function, e.g. 'add()`? This would:

  1. Allow more flexibility in how it's defined without worrying about backwards compat.
  2. There are other use cases that could be covered, then. E.g. notice how adding an item to a nested set is also verbose in examples above.

itamarst avatar Feb 07 '17 17:02 itamarst