kalekundert / byoc

MIT License
0 stars 0 forks source link

Allow `Config`-specific `Key` keyword arguments #15

Closed kalekundert closed 3 years ago

kalekundert commented 3 years ago

The specific example I have in mind is the Po4Config, which looks up values for a list of tags (specified by the object) in a database. This config has a class-wide pick setting to specify if/how these values should be condensed into a single value, but this should really be a per-parameter setting. In general, I think it would be useful to have a mechanism for Configs to incorporate extra information when looking up values.

The natural way to provide this information is via the Getter class. Getter already supports similar functionality for nonstandard parameter types (e.g. toggle_param()). This is a little different because config keyword arguments would only apply to Key, not to the other Getter classes.

The trick is figuring out how to make use of the keyword arguments when looking up values from the config. The problem is that values are looked up using __getitem__(), which only supports a single positional argument. Some ways to accomodate the extra information:

  1. Wrap Layer.values in an object. Not great because it makes it hard to access the values object using function keys.
  2. Delegate value access to the Layer class, such that it can then be subclassed and modified. I'm not crazy about this because this doesn't really pertain to the layer: it only really pertains to the value. I also feel like allowing the Layer to customize value access could potentially be confusing.
  3. Call __getitem__() explicitly with keyword arguments. I can't provide keyword arguments to __getitem__() via the [] syntax, but I can just call the method with whatever arguments I want. So I can query the Config class (which Key has direct access to) to see if it expects any keyword arguments, and provide them if necessary. One problem is that the keyword arguments would not get used with function keys.
  4. Give Config a method which "prepares" the values. Specifically, this method would be given the values object and any keyword arguments. It would return a context manager, which would be entered before looking up a specific value and exited afterwards. The enter function would return the values that will actually be used for the lookup, which could be different than the values provided to the method in the first place. The drawback with this implementation is that the keyword arguments become state, which adds complexity w.r.t. setting/unsetting the state (hence the context manager) and keeping in mind that multiple objects might share the same Config/values instance. However, I think this complexity is unavoidable, because the keyword arguments need to be stateful in order to support function arguments (e.g. where the caller has control over how the lookup is done).

I like (4). It seems like a fairly powerful and general API.

kalekundert commented 3 years ago

I don't know why I didn't realize this in the first place, but I can just use the existing cast argument for the Po4Config application described above. In the absence of a compelling use-case, I'm going to close this issue.