Patch: fix __module__ for instrumented attribute
If {{{Foo}}} is a class and {{{Foo.bar}}} is an attribute of that class, it is reasonable to assume that {{{Foo.bar.module == Foo.module}}}.
Attached is a simple patch for this issue along with a test case. As it doesn't apply cleanly to sa05, and a backport would make me very happy, I have included a patch for both trunk and sa05.
For what it's worth, Sphinx is one tool that operates under the above assumption and this patch will fix an [issue with generating documentation for model classes](http://groups.google.com/group/sphinx-dev/browse_thread/thread/1be84427fad69529/d2f568d5ef9f306a).
Comments (9)
-
Account Deleted -
Account Deleted Patch for sa05
-
Account Deleted Just realized that the sa05 patch is probably bogus, I didn't use hg merge to make it but just applied the diff with "patch"... I've never worked with mercurial before so not sure how to do it right. Sorry about that.
-
This sounds like a sphinx issue. The assumption that Class.module == Class.prop.module is spurious- it's not part of the descriptor protocol or Python class composition in general.
For example, in the builtins, @property doesn't set a module on its descriptor instance, classmethod(otherpackage.function) reports a module of otherpackage on the descriptor.
Information on why this is change required in this instance is needed, plus ensuring that masking the module does not break source code reporting in pdb or other debuggers.
-
repo owner The change in e1c47d12bf871bf30a149c7fc167d5d22ddaa222 removes the generation of the "default" docstring by just setting doc to
None
if not otherwise present. I suppose I'm only using ..automodule and ..autoclass so far. I don't necessarily agree that__module__
should be on descriptors - the descriptor implementation does come from a different module and it feels wrong to conceal that. -
Account Deleted Fair points, it looks like I'll have to do some more digging.
For now I'd like to point out that {{{Table.attr.class}}} is still {{{InstrumentedAttribute}}}, {{{Table.attr.class.module}}} is still {{{sqlalchemy.orm.attributes}}}, and raising an exception in {{{InstrumentedAttribute}}} code still shows the correct source information after this change. So I'm not sure that you can say this change conceals anything.
I'll test source code reporting in pdb when I find some time later on, and I'll also see if @property also breaks Sphinx. If so, that would probably be a good indicator that this should indeed be fixed in Sphinx.
-
repo owner - changed milestone to blue sky
-
Account Deleted You are right, this problem is better fixed in Sphinx. Please close this ticket, sorry for the noise...
For what it's worth, I've filed a ticket for the Sphinx issue here: http://bitbucket.org/birkenfeld/sphinx/issue/568/autoattribute-cant-extract-docstring-for
-
repo owner - changed status to wontfix
- Log in to comment
Patch for trunk