Is there any specific reason that @attr.s doesn't add at least __getitem__ and __setitem__?
attr.asdict() is nice, but direct attribute access by key would be even more convenient.
For example, instead of:
backend = get_backend(config)
while not backend.check_config():
click.echo('Invalid Configuration parameters!')
for param_name, question in backend.config.check_config_requires:
value = click.prompt(question, default=getattr(backend.config, param_name))
setattr(backend.config, param_name, value)
would be much nicer to write:
backend = get_backend(config)
while not backend.check_config():
click.echo('Invalid Configuration parameters!')
for param_name, question in backend.config.check_config_requires:
value = click.prompt(question, default=backend.config[param_name])
backend.config[param_name] = value
It thought this syntax would be nice (same as attr.s(repr=True)):
@attr.s(dict=True)
class VaultConfig:
name = 'Vault'
url = attr.ib(default='http://localhost:8200')
Sorry but asdict is exactly intended for your use case of easy iteration on attributes (and json.dumps ;)).
Generally speaking I鈥檓 trying to keep the impact of attrs on _your_ classes as small as possible & put functionality into separate functions.
I don't think this would be such a big deal, but you should consider at least the __iter__ method from my PR (#55), which would make dict(c) possible which is a much nicer api than attr.asdict(c) IMO.
Also if it would be False by default it would not have any impact, but would make it convenient for someone who needs it.
Adding things like __iter__ and __getitem__ expands the public interface of every attrs-using object quite a bit. It also enables things like slicing, which are undesirable. If attrs did this I would want to turn it off 99% of the time.
Also, that is a _horrible_ way to initialize an object's configuration. You start with an incompletely initialized, invalid object, and then you directly access some of its state, which gives you a list of questions to ask, and then you stuff the answers back on as attributes? How about something like this instead:
def backend_from_file_with_questions(config_file, click):
config = {}
defaults = load_file(config_file)
config.update(defaults)
while True:
try:
return get_backend(config)
except IncompleteConfiguration as ic:
click.echo('Invalid Configuration parameters!')
for param_name, question in ic.unanswered_questions:
default = defaults.get(param_name, "")
config[param_name] = click.prompt(question, default=default)
With this approach:
config dict is a grab-bag of course, but it's a dict, and the process of validating it either works or doesn't.config as an attribute of backend at all - and indeed it _really_ shouldn't be; it's not the application's business what the configuration of the backend, the configuration is presumably there to facilitate the backend fulfilling a task. (in other words: backend.config.__setitem__ is a law of demeter violation)config object _at all_ if you can help it; backend just has whatever attributes, private or public, that it needs to do its job.You can still use attrs to help you declare backend, of course, but there's no reason that you need a config object at all.
An even better approach would be to create a stateful intermediate BackendBuilder to manage the dialog so you can validate the configuration one question at a time, and provide more immediately actionable feedback to the user in the UI. But that starts to go a little beyond the scope of this issue.
Most helpful comment
Adding things like
__iter__and__getitem__expands the public interface of everyattrs-using object quite a bit. It also enables things like slicing, which are undesirable. Ifattrsdid this I would want to turn it off 99% of the time.