snakebids icon indicating copy to clipboard operation
snakebids copied to clipboard

Improve implementation of `get()` in `MultiSelectDict`

Open pvandyken opened this issue 2 years ago • 2 comments

I was confronted with a current limitation of our MultiSelectDicts: it is currently somewhat difficult to select only the keys that are contained inside the dict. You need something like this:

inputs['FA'].wildcards[
    tuple({"subject", "session"} & inputs["FA"].wildcards.keys())
])

If one of the items in the tuple is not found in the dict, you (sensibly) get a KeyError. So we prefilter our keys by the keys found in the dict to avoid the error.

In normal dicts, you have .get() to retrieve keys which may or may not be present. Its optional behaviour does not extend to tuples, however. In fact, MultiSelectDict.get() does not work with tuples at all!

I would propose the following behaviour:

If a string (the only valid key type currently) is passed to .get(), the behaviour is exactly the same as currently.

If a tuple is passed, then the method works with the same semantics as a defaultdict. Instead of default as the secondary argument, we have (the implicitly named) default_factory. As in defaultdict, it may be given either a 0-arg callable or None.

  • If None, then only the keys found in the parent dict will be returned in the child dict. If none of the keys are found, you get an empty dict.
  • If a callable, then keys not found in the parent dict will be lazily populated with the result of the callable.
  • Unlike a defaultdict:
    • Unlike a defaultdict, only keys originally passed to the get method will be valid. Other arbitrary keys will always give a key error.
    • The keys passed in the construction of the dict will be immediately deemed "present". In other words, querying key in child_dict will return True
  • The logic above applies to nested .get calls. If a key is "added" to a MultiSelectDict via a .get call, it will be deemed present in subsequent calls to .get

pvandyken avatar May 15 '23 20:05 pvandyken

Given that there's a straightforward workaround, we'll leave this out of 1.0 while we decide on the implementation.

tkkuehn avatar Jun 02 '23 17:06 tkkuehn

Just wanted to note a problem with the current workaround is that one cannot easily control the order of the keys. This has come up as a problem for me in one application.

pvandyken avatar Sep 06 '23 15:09 pvandyken