[merge] Registry

[Aaron Bentley]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Martin Pool wrote:

> +1, except for a couple of grammatical fixes.  You have a habit of
> separating your clauses into sentences, as in this quoted section.  I
> don't mind how you write your email, but we should try to stick to
> standard English in documentation, and I'm pretty sure this isn't
> standard.

Can we also get hardcore about apostrophes?  There's a certain core dev
who often omits them, and I always find that contractions which are
missing their apostrophes are quite jarring.  "dont" and "cant" are
acceptable in symbol names, but I don't think they belong in doctexts or
comments.  (Though "cant" is a real word that means something else.)

Getting more serious about grammar will make it a bit harder to merge
patches.  We might want to consider making integrators take
responsibility for tweaking grammar, because I don't want this to become
an extra hoop for non-native English speakers to jump through.

(I can't really come up with an inoffensive way of phrasing this.  Not
only do some non-native speakers have superlative grammar, but some
native speakers have quite poor grammar.  Anyhow, I've got good
intentions, I swear.)

>> +    This is used by Registry to make plain objects function similarly
>> +    to lazily imported objects.

I'd be inclined to use hyphens for compound words like
"lazily-imported", which makes the binding crystal-clear.

>> +    Objects can be any sort of python object (class, function, module,
>> +    instance, etc)
>> +    """
>> +
>> +    __slots__ = ['_obj']
>> +
>> +    def __init__(self, obj):
>> +        self._obj = obj
>> +
>> +    def get_obj(self):
>> +        """Get the object that was saved at creation time"""
>> +        return self._obj
>> +
>> +
>> +class _LazyObjectGetter(_ObjectGetter):
>> +    """Keep a record of a future object, and on request load it."""

Should be "and, on request, load it."

>> +
>> +    __slots__ = ['_module_name', '_member_name', '_imported']
>> +
>> +    def __init__(self, module_name, member_name):
>> +        self._module_name = module_name
>> +        self._member_name = member_name
>> +        self._imported = False
>> +        super(_LazyObjectGetter, self).__init__(None)
>> +
>> +    def get_obj(self):
>> +        """Get the referenced object.
>> +
>> +        If not imported yet, this will import the requested module.

This has the object of the sentence hidden at the end of it.  I'd
prefer: "This will import the requested module, if not yet imported"

>> +        :param help: Help text for this entry. This may be a string or
>> +                a callable. If it is a callable, it should take two
>> +                parameters, this registry and the key that the help was
>> +                registered under.

This should have a colon rather than a comma after 'parameters',
because the list 'this registry and the key...' describes 'parameters'.
To demonstrate the difference:

a) On my trip, I took two pets, a seeing-eye dog and my husband.
b) On my trip, I took two pets: a seeing-eye dog and my husband.

In a), the two pets are not described.  The dog and the husband are in
addition to the pets.

In b), the two pets are the seeing-eye dog and the husband.

The way it is now is understandable, but not as clear and correct as it
could be.

>> +        :param info: More information for this entry. Registry.get_info()
>> +                can be used to get this information. It is meant as an
>> +                opaque storage location.
>> +        :param override_existing: If True, replace the existing object

Should this be "If True, replace any existing..." ?

Also, "is meant as" might be better as "should be treated as".

>> +    def register_lazy(self, key, module_name, member_name,
>> +                      help=None, info=None,
>> +                      override_existing=False):
>> +        """Register a new object to be loaded on request.
>> +
>> +        :param module_name: The python path to the module. Such as 'os.path'.
>> +        :param member_name: The member of the module to return, if empty or 
>> +                None get() will return the module itself.

This is a comma splice.  A semicolon, if not a period, should be here,
because there are two thoughts.  I'd write:

:param member_name: The member of the module to return.  If empty or
        None, get() will return the module itself.

>> +        :param help: Help text for this entry. This may be a string or
>> +                a callable.
>> +        :param info: More information for this entry. Registry 
>> +        :param override_existing: If True, replace the existing object
>> +                with the new one. If False, if there is already something
>> +                registered with the same key, raise a KeyError

I think "replace *any* existing object with the *provided one*" might be
clearer than "the new one".

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFFLkYH0F+nu1YWqI0RAjo4AJ9/a03NMQorD5CkjVooZ0p78TWxbQCeKe5K
/SXDwuFaqAH4VDkOwdu7CYY=
=nO4F
-----END PGP SIGNATURE-----