-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Test and fix for multidicts' .items(), .keys() and .values() repr #413
Conversation
@@ -305,6 +305,9 @@ def __init__(self, items): | |||
def __len__(self): | |||
return len(self._items) | |||
|
|||
def __repr__(self): |
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 guess _multidict.pyx
also need __repr__
for _ViewBase
.
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.
yes, for sure)
I must say that test_muldict.py
is almost as complicated as aiohttp parsers...
Can you point me where should I put tests for cython's multidict?
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.
Good question. I suggest pushing along with test___repr__
: https://github.com/KeepSafe/aiohttp/blob/master/tests/test_multidict.py#L269
Maybe I should refactor multidicts' test hierarchy.
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.
tests already there
9ba55a1
to
9bce9e5
Compare
Also fixed a typo in |
Test and fix for multidicts' .items(), .keys() and .values() repr
Thanks! |
abc.ItemsView / KeysView / ValuesView
has__repr__
method but it expects that data is stored in._mapping
attribute.First commit is failing test, second -- the fix.