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
Update documentation and Provides repr for better debugging. #234
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly LGTM.
self.assertEqual( | ||
repr(inst), | ||
"<zope.interface.Provides for %r>" % type(self) | ||
"<zope.interface.Provides for instances of %r providing %s>" % ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer to see the complete text here. Is this possible?
Currently the test seems to be is nearly a repetition of the code in the method under test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great point. I'll make a change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given spec = Provides(AClass, IFoo, IBar)
, the repr will be something like
<zope.interface.Provides for instances of <class 'some.module.AClass'> providing (<InterfaceClass some.module.interfaces.IFoo>, <InterfaceClass some.module.interfaces.IBar>)>
Informative, but not terse.
Basically none of these objects have a __str__
method.
__repr__
is meant to "look like a valid Python expression that could be used to recreate an object with the same value (given an appropriate environment)."
In contrast __str__
computes "the 'informal' string representation of an object. This differs from __repr__()
in that it does not have to be a valid Python expression: a more convenient or concise representation may be used instead. The return value must be a string object."
For Provides
, it would be pretty easy to give a repr
that actually can be evaluated (in many cases) outputting something like
Provides(AClass, IFoo, IBar)
Similarly, interfaces could have a __str__
that just outputs __module__.__name__
.
That would transform reports from
Object <InterfaceClass BTrees.Interfaces.ISet> has different legacy and C3 MROs:
Legacy RO (len=7) C3 RO (len=7; inconsistent=no)
====================================================================================================
<InterfaceClass BTrees.Interfaces.ISet> <InterfaceClass BTrees.Interfaces.ISet>
<InterfaceClass BTrees.Interfaces.IKeySequence> <InterfaceClass BTrees.Interfaces.IKeySequence>
- <InterfaceClass BTrees.Interfaces.ISized>
<InterfaceClass BTrees.Interfaces.ISetMutable> <InterfaceClass BTrees.Interfaces.ISetMutable>
<InterfaceClass BTrees.Interfaces.IKeyed> <InterfaceClass BTrees.Interfaces.IKeyed>
<InterfaceClass BTrees.Interfaces.ICollection> <InterfaceClass BTrees.Interfaces.ICollection>
+ <InterfaceClass BTrees.Interfaces.ISized>
<InterfaceClass zope.interface.Interface> <InterfaceClass zope.interface.Interface>
to
Object <InterfaceClass BTrees.Interfaces.ISet> has different legacy and C3 MROs:
Legacy RO (len=7) C3 RO (len=7; inconsistent=no)
==================================================================================
BTrees.Interfaces.ISet BTrees.Interfaces.ISet
BTrees.Interfaces.IKeySequence BTrees.Interfaces.IKeySequence
- BTrees.Interfaces.ISized
BTrees.Interfaces.ISetMutable BTrees.Interfaces.ISetMutable
BTrees.Interfaces.IKeyed BTrees.Interfaces.IKeyed
BTrees.Interfaces.ICollection BTrees.Interfaces.ICollection
+ BTrees.Interfaces.ISized
zope.interface.Interface zope.interface.Interface
(With similar benefits when Provides
is present.)
Since the changes in Provides
here mean that there might have to be doctest updates, and since adding a __str__
mostly shouldn't (but might!) cause doctest updates, this might be a good time to make such a change.
What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved discussion to issue #236.
9967fcc
to
f46bc4f
Compare
Thanks for the review! |
Fixes #229.
Replaces #232