Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Docs: show off "dot notation" attribute access when validating an entire dict at once #69

Open
beasteers opened this issue Feb 27, 2020 · 7 comments

Comments

@beasteers
Copy link
Contributor

beasteers commented Feb 27, 2020

It would be much prettier to be able to do config.something[0].name.get() instead of using string keys for everything. I realize there's the concern of keys being masked by object attributes, but I feel like it's worth it (you can still use strings in those cases). And that conflict could be partially be alleviated by adding an API reference so people will be able more aware of keys that will be masked.

@sampsyo
Copy link
Member

sampsyo commented Feb 28, 2020

Hi! Yeah, I have often thought this as well. I'm certainly interested in exploring it if anyone wants to drive the effort—as you say, the primary challenge will be avoiding conflicts with actual attributes (i.e., you can't use this to address a configuration attribute called get!).

Or we could explore a second interface just for doing dot-based access that minimizes the number of "real" attributes in favor of global functions… as in confuse.as_str(config.something[0].name) instead of config.something[0].name.as_str().

@beasteers
Copy link
Contributor Author

Well it could be as straight forward as:

In [1]: class Config(dict): 
   ...:     def __getattr__(self, *a, **kw): 
   ...:         return self.__getitem__(*a, **kw) 
   ...:                                                                                                                    

In [2]: cfg = Config()                                                                                                     

In [3]: cfg['a'] = 5                                                                                                       

In [4]: cfg.a                                                                                                              
Out[4]: 5

In [5]: cfg.b                                                                                                              
---------------------------------------------------------------------------
KeyError                                  Traceback (most recent call last)
<ipython-input-5-7f8d0c2a206f> in <module>
----> 1 cfg.b

<ipython-input-1-96bda7256865> in __getattr__(self, *a, **kw)
      1 class Config(dict):
      2     def __getattr__(self, *a, **kw):
----> 3         return self.__getitem__(*a, **kw)
      4 

KeyError: 'b'

The only thing to change is KeyError -> AttributeError (if you want to be proper).


I honestly don't think it's that much of a problem to have real attributes masking the config ones (especially for things like get, add). I think as long has you have a full API Reference (autogenerated of course) which lists all of the config object methods that a user needs to avoid, then I think you're good. The point isn't to replace key notation so it doesn't have to be cover all cases, just be prettier.

To be extra cautious, you can add a notice above that part of the documentation saying that keys like get, set, str access the config methods and must be accessed via key notation (config.something['get'].sdfg[0].adsf).

As a less problematic option could be: cfg['asdf.0.afds.name'], but like who knows, maybe people use dots in their keys (or ints as strings).

This isn't a huge deal, it's mostly cosmetic, but unnecessary str keys bother me a little bit idk why lol.

@sampsyo
Copy link
Member

sampsyo commented Feb 29, 2020

Sure. But on the other side of the argument, this will make some aspects of development worse—for example, if you make a typo like config.foo.as_sstr(), the error message will be much more confusing, and we'll lose the ability to do mypy type checking on config view objects.

I do think a natural way to do this might be to make it optional. That is, you could have a DotConfig object or whatever, together with a DotView, that permits this type of access. Because it's opt in, we can sort of have the best of both worlds.

@beasteers
Copy link
Contributor Author

beasteers commented Apr 20, 2020

as an update - I recently started using confuse more extensively on a project and I realized that you can do:

cfg = config.get({
    'data_root': confuse.Filename(),
    'upload': {
        'server': 'localhost:8085',
        'interval': 3
    }
})

print(cfg.upload.server)

which frankly is genius and is much more comfortable than calling .as_str() throughout your code anyways. (nice work 😊)

I think this design pattern is better than anything that would come out of mangling attribute getters so I'd say we can probably close this issue. 🎉


Although - an example like this should probably be featured more prominently in the docs, describing the quirks of setting default values/validation types.

@sampsyo
Copy link
Member

sampsyo commented Apr 21, 2020

Oh yeah!! I had forgotten that we had that very nifty AttrDict trick in the output. Maybe we need to document that better…

@beasteers
Copy link
Contributor Author

Yeah! I came across it in https://github.com/beetbox/confuse/blob/master/example/__init__.py

honestly I feel like this should be the recommended interface. Especially because it does all of the validation once at the top and wraps it into one object.

But yeah it would be great to have a few examples in the docs!

@sampsyo
Copy link
Member

sampsyo commented Apr 21, 2020

Yeah, it's the right thing for many use cases. One of the original sparks for the design of the library was that we could allow validation at different granularities: individual fields when you want it, or entire config files when you want that, or anywhere in between. In large applications (it was written for beets), it's critical to be able to chop up the configuration and validate it in pieces, distributed throughout the code, rather than relying on one "god module" to do all the validation up front. So we end up doing validation at several granularities in different parts of the system.

Anyway, I'll reopen this issue and retitle it as a reminder to improve the docs in this way…

@sampsyo sampsyo changed the title Dot notation Docs: show off "dot notation" attribute access when validating an entire dict at once Apr 21, 2020
@sampsyo sampsyo reopened this Apr 21, 2020
@sampsyo sampsyo mentioned this issue Jun 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants