Add resolving string to NestedChainMap

[teutoburg]

This is in preparation for solving AstarVienna/ScopeSim#387

A resolving string is a string key ending in "!". An instance of NestedChainMap will only continue to follow references if the key is a resolving key, otherwise it will return the value a the bang-string that is stored.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.31%. Comparing base (f049cd5) to head (42af98a).
Report is 6 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main      #47   +/-   ##
=======================================
  Coverage   99.30%   99.31%           
=======================================
  Files           6        6           
  Lines         434      435    +1     
=======================================
+ Hits          431      432    +1     
  Misses          3        3           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[teutoburg]

For values that point to other values, this will now only continue resolving when the value itself is a resolving string. Consider the following example:

ncm = NestedChainMap(
    RecursiveNestedMapping({"foo": {"a": "!foo.b"}}),
    RecursiveNestedMapping({"foo": {"b": "!foo.a"}})
)
ncm["!foo.a!"]  # '!foo.a'

ncm = NestedChainMap(
    RecursiveNestedMapping({"foo": {"a": "!foo.b!"}}),
    RecursiveNestedMapping({"foo": {"b": "!foo.a"}})
)
ncm["!foo.a!"]  # '!foo.b!'

ncm = NestedChainMap(
    RecursiveNestedMapping({"foo": {"a": "!foo.b"}}),
    RecursiveNestedMapping({"foo": {"b": "!foo.a!"}})
)
ncm["!foo.a!"]  # '!foo.a!'

ncm = NestedChainMap(
    RecursiveNestedMapping({"foo": {"a": "!foo.b!"}}),
    RecursiveNestedMapping({"foo": {"b": "!foo.a!"}})
)
ncm["!foo.a!"]  # RecursionError (as expected)

Now the question is: Is that what we want? Or should things always be resolved when the original key is a resolving one?

[teutoburg]

Also, this now only affects the NestedChainMap. Anything inside a single RecursiveNestedMapping will still be resolved regardless. Again, is that what we want? Within the context of AstarVienna/ScopeSim#387, it seems to work fine the way it is now in this PR. @hugobuddel, what say? Just do it now this way and if we encounter problems elsewhere, we can always come back and change how it works?

[hugobuddel]

I see the ! only as a syntax to tell the resolver to resolve the key, but not as part of the key itself. So having a trailing ! in a value doesn't make sense to me. So of your above 4 examples, I think that only this one should be allowed:

cm = NestedChainMap(
    RecursiveNestedMapping({"foo": {"a": "!foo.b"}}),
    RecursiveNestedMapping({"foo": {"b": "!foo.a"}})
)

And all the others should not be allowed to exist. Maybe that disallows string parameters that start and end with an exclamation mark, but who cares.

And then I would expect

ncm["!foo.a!"]

to fully resolve until it reaches a value, so in this case a RecursionError should be raised.

However, I also think that it doesn't make sense to start a key with a parameter like that. One should just do

ncm["foo.a!"]

and have it fully resolve.

To me, the idea behind it is that it gives the user (and us developers) an easy way to fully resolve a parameter.

[teutoburg]

Agree with your reasoning 👍

I didn't explicitly disallow strings ending in ! as values (for now), I don't think that's quite necessary.

Also, this now only affects the NestedChainMap. Anything inside a single RecursiveNestedMapping will still be resolved regardless. Again, is that what we want? Within the context of AstarVienna/ScopeSim#387, it seems to work fine the way it is now in this PR. @hugobuddel, what say? Just do it now this way and if we encounter problems elsewhere, we can always come back and change how it works?

Since you didn't object I'm assuming you're fine with "let's keep it as it is (for now)" 🙂

[hugobuddel]

Also, this now only affects the NestedChainMap. Anything inside a single RecursiveNestedMapping will still be resolved regardless. Again, is that what we want?

I think I perhaps didn't understand the point. Do you mean that

rnm = RecursiveNestedMapping({"foo": "a", "bar": "!foo"}})
rnm["bar"] == "a"
rnm["bar!"] == "a"

If, so, I do object, I think

rnm = RecursiveNestedMapping({"foo": "a", "bar": "!foo"}})
rnm["bar"] == "!foo"
rnm["bar!"] == "a"

but I don't think this is something we can do in ScopeSim?

Because it should be easy to get to the "!foo" value.

[teutoburg]

but I don't think this is something we can do in ScopeSim?

Why not?

I did it (using the same implementation now for both classes, some refactoring due... a mixin class...), and it seems to work fine (ScopeSim tests pass with it). I added your example as a test (after fixing the rogue } 😛 )

[hugobuddel]

but I don't think this is something we can do in ScopeSim?

Why not?

I meant like in the IRDB. Well we probably can, but we never do that. There is never a "!temperature" or something like that in the yaml files, always "!INST.temperature" etc. I guess it is fine to support it though.

The tests as they are now are what I would expect, so I think we can go for this.

[hugobuddel]

Great solution, especially since we apparently already document it.