Hi Steven,
Thank you for your response!
On Apr 11, 4:22 am, Steven D'Aprano <st...@remove-this-
cybersource.com.au> wrote:
> On Fri, 10 Apr 2009 19:04:38 -0700, Edd wrote:
> > Hi folks,
>
> > I'd like to use Python itself as the configuration language for my
> > Python application. I'd like the user to be able to write something like
> > this in their config file(s):
>
> > cfg.laser.on = True
> > cfg.laser.colour = 'blue'
> > cfg.discombobulated.vegetables = ['carrots', 'broccoli'] # ...
>
> > To this end, I've created a class that appears to allow instance
> > variables to be created on the fly.
>
> Um, don't all classes allow that?
>
> >>> class MyClass(): pass
> ...
> >>> instance = MyClass()
>
> Or do you mean instance *attributes*? Again, apart from built-in types
> and classes that use __slots__, all classes allow that.
>
> >>> instance.parrot = 'Norwegian Blue'
Yes I probably mean instance attributes. Forgive me, I am not
particularly sure of the terminology. But your MyClass example, won't
quite do what I want, as I'd like to be able to define instance
attributes on top of instance attributes by assignment:
>>> class MyClass(): pass
...
>>> instance = MyClass()
>>> instance.lasers.armed = True
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
AttributeError: MyClass instance has no attribute 'laser'
>>>
> > In other words, I can to the
> > following to read a config file:
>
> > cfg = Config()
> > execfile(filename, {'cfg', cfg}, {})
>
> That's okay so long as you trust the user not to put malicious, or buggy,
> code in your config file. Personally, I think config files should be more
> tolerant of errors than a programming language.
That's certainly a valid remark, but this will be a tool for
programmers. I am hoping that the user will make use of the power in
moderation. Often, it really will be useful to allow functions to be
defined in the config files, for example.
> > However, I think my implementation of the Config class is a little
> > crappy. I'd really appreciate the critical eye of a pro. Here's the
> > sauce:
>
> For starters, where is your documentation? No doc strings, not even any
> comments! No, I tell a lie... *one* obscure comment that doesn't really
> explain much.
Yes, you're quite right. I was about to add some doc strings, but I
didn't think the implementation was good enough. That's somewhat
backwards, though, right?! Especially considering I'm asking for
improvements. Anyway, I had hoped that the example usage at the end
would show what the purpose of the class is.
>
> > class Config(object):
> > def __init__(self, sealed=False):
> > def seal():
> > for v in self._attribs.values():
> > if isinstance(v, self.__class__): v.seal()
> > del self.__dict__['seal']
>
> > d = {'_attribs': {}, '_a2p': None}
> > if not sealed: d['seal'] = seal
>
> > self.__dict__.update(d)
>
> I'm going to try to guess what the above does. When you initialise an
> instance, you can tell the instance to be "sealed" or unsealed. I'm not
> sure what the difference is, or why you would choose one over the other.
> Sealed instances seem to be exactly the same as unsealed instances,
> except they have a seal() method (actually a closure). The seal method,
> when called, recursively seals any embedded Config instances inside the
> current instance, then deletes itself.
>
> Arghhh!!! Self-modifying code!!! Unclean, unclean!!!
Quite!
> I'm not sure why seal() is necessary -- it seems to me that if present,
> all it does is delete itself. So why not just leave it out altogether?
As I said in the original post, such Config objects will be made
available to other kinds of user-written script and it's important
that the Config not change between the execution of one script and the
next. The seal() mechanism was an attempt to help the user from
*accidentally* doing this and then having to try to diagnose the
problem and understand how changing the config might have broken the
invariants of the software. I guess a big "DON'T CHANGE THE CONFIG IN
YOUR SCRIPTS" message in the manual, might be sufficient, though :)
> You also have a rather complicated way of adding instance attributes.
> Instead of
>
> d = {'_attribs': {}, '_a2p': None}
> self.__dict__.update(d)
>
> why not just do the more obvious:
>
> self._attribs = {}
> self._a2p = None
Because that would go through __setattr__(), which does something else
(which is the whole point of the class). At least, that was my
understanding, which certainly could be at fault.
This might be nicer I guess:
self.__dict__['_attribs'] = {}
self.__dict__['_a2p'] = None
There used to be more instance attributes than just two so it was
easier to put them in a dict and use update. I agree that it's rather
obfuscated, though.
[Edd's horrendous code snipped]
> It looks like you are just re-inventing the normal attribute mechanism of
> Python. I'm not sure why you feel this is necessary. And it contains MORE
> self-modifying code! Yuck! Frankly I don't care enough to dig into your
> code to understand how it works in detail.
Ok. But is there a quick-and-easy way of creating an object, cfg, such
that I can write:
cfg.hovercraft.full.of = 'eels'
without knowing in advance that the user will want a .hovercraft
instance attribute, or a .full attribute inside that, or a .of inside
that, or ... ?
> > def __setattr__(self, key, value):
> > if key in self.__dict__:
> > self.__dict__[key] = value
> > else:
> > if not 'seal' in self.__dict__:
> > clsname = self.__class__.__name__
> > raise AttributeError("'%s' object attribute '%s'
> > is read-only (object is sealed)" % (clsname, key))
> > self.__dict__['_attribs'][key] = value if self._a2p:
> > self._a2p()
> > self._a2p = None
>
> Does "sealed" mean that the instance is read-only? If so, and if I'm
> reading this correctly, I think it is buggy. You allow modifications to
> attributes inside __dict__ *without* checking to see if the instance is
> read-only. Then you get the test backwards: surely the existence, not the
> absence, of a 'seal' attribute should mean it is sealed?
The absence of the seal() 'method' means that it has already been
called i.e. the object has already been sealed. I agree it's somewhat
fishy, which is why I'm asking for suggestions for improvements.
Perhaps I should just forget the seal() idea.
> > Once the config is loaded, it will be passed down to other user- written
> > scripts and it's important that these scripts don't accidentally change
> > the config. So the idea is that I'll call cfg.seal () to prevent any
> > further changes before passing it on to these other scripts.
>
> Or you could pass a *copy* of the config, and let them change it to their
> heart's content, it won't matter.
I think that's probably the best thing. I was worried about some parts
not being deep-copyable, but I think I'm happy to put that concern
aside. Besides, right now, even though you can't add/delete attributes
in a Config, you can still change existing ones:
cfg = Config()
cfg.a.b.c = [1, 2]
cfg.seal()
cfg.a.b.c[1] = 3 # pffff
Yes, you've helped convince me that it's just a bad idea.
> Or you could say "we're all adults here", simply document that any
> changes will have consequences, and let user scripts change the config.
> And why not?
Even adults make the occasional tiny mistake which has confusing
consequences in a larger system. It was an attempt to help prevent
this. I probably worry too much about that.
> > Beyond the
> > general fiddliness of the code, I think the way seal() currently works
> > is particularly pants.
>
> Is "pants" slang for "fragile, hard to understand and difficult to debug"?
Yes! Rest assured that I am under no illusion that what I have written
is good!
Steven, I greatly appreciate your taking the time to understand the
aforementioned horrors. Assuming that the seal() stuff is no longer a
requirement, is there a cleaner way of creating an object where
(nested) instance attributes can be defined by simple assignment?
Perhaps it would have been better if I had left out my awful attempt
altogether. It seems like it only made my question more confusing than
it needed to be :( If it would help, I'd be happy to add some doc
strings and tests but I was getting ready to throw this code away when
a cleaner 5-line alternative was presented!
Kind regards,
Edd
--
http://mail.python.org/mailman/listinfo/python-list
