kalekundert / byoc

MIT License
0 stars 0 forks source link

Better error message when configs are out of order #31

Open kalekundert opened 2 years ago

kalekundert commented 2 years ago

Consider this script, which tries to read a parameter from a config file specified on the command-line:

import byoc
from byoc import Key, DocoptConfig, NtConfig

class MyApp:
    """\
Usage:
    my_app [-x <path>]
"""
    __config__ = [
        DocoptConfig,
        NtConfig.setup(path_getter=lambda self: self.x)
    ]
    x = byoc.param(Key(DocoptConfig, '-x'))
    y = byoc.param(Key(NtConfig, 'y'))

app = MyApp()
byoc.load(app, DocoptConfig)

app.y

It has a bug, which is that the NtConfig is loaded before the DocoptConfig, so the path getter will never be able to successfully get a value from self.x. However, the error message isn't very clear about this:

byoc.NoValueFound: can't find value for parameter
• getting 'y' parameter for <__main__.MyApp object at 0x7fb790474910>
• skipped NtConfig(): loaded, but no layers
• failed to get path(s):
  raised NoValueFound: can't find value for parameter
  • getting 'x' parameter for <__main__.MyApp object at 0x7fb790474910>
  • no configs of class DocoptConfig
  • did you mean to provide a default?
• did you mean to provide a default?

The problem is "no configs of class DocoptConfig". That's confusing because the class does have a DocoptConfig, it just isn't available while NtConfig is being loaded. I should change this message to distinguish between "this class doesn't even have that kind of config" and "this config isn't available at the moment due to the order the configs are being loaded in".

kalekundert commented 2 years ago

In fact, looking into this, I think there might be a bigger bug in the code.

When loading, model._load_configs() basically empties the wrapped configs list and builds it back up one element at a time. But param._calc_bound_getters() can depend on the length of that list, e.g. to work out implicit keys. I don't see how this could work, and don't know if I have test cases for reloading implicit keys.

In either case, probably what I should do is always keep the wrapped configs list always the same size, and have a flag that indicates whether the wrapped config is available at the moment. That would allow BoundKey to distinguish these two cases, and avoid any problems with reloading implicit keys.