Gen.jl icon indicating copy to clipboard operation
Gen.jl copied to clipboard

Make choicemaps internally handle keys in a uniform way (move special treatment of `Pair`s closer to the surface)

Open bzinberg opened this issue 4 years ago • 0 comments

Choicemaps have a notion of "submap," and define a specialization of Base.setindex!(choicemap, k, v) allowing the user to assign the value v to the key k. Currently, the behavior of this method depends on the type of k: a submap is created iff k is a Pair. Consequently, afaik, there is no way to assign a value to a Pair-valued key without having a submap.

It seems to me that the complexity of recursively destructuring pairs can/should be confined closer to the surface -- e.g., in the most commonly used accessor (where there is also a more systematic counterpart that perhaps requires a bit more boilerplate to use). I'd say in general, systematic internals with convenient helper functions at the surface gets the best of both worlds for maintainability and user convenience.

We probably won't want to break existing usage of Pairs anytime soon, for backward-compatibility. But I think a backward-compatible change and potentially a deprecation are still warranted.

One possibility is,

we could create a type called KeyPath which is just a wrapper around a linked list, and have a method

get_at_keypath(::ChoiceMap, kp::KeyPath)

which walks the choicemap as indicated. The difference is that a caller of this method knows kp is being treated as a keypath, even if they are writing generic code that populates kp with elements of unknown type. And, if kp contains only one element, they know that the lookup will not recurse into submaps.

Thoughts?

bzinberg avatar Apr 12 '21 21:04 bzinberg