On 23.9.2013 12:08, Petr Viktorin wrote:
On 09/23/2013 09:18 AM, Jan Cholasta wrote:
On 18.9.2013 14:00, Petr Viktorin wrote:
On 09/17/2013 05:13 PM, Jan Cholasta wrote:
On 20.2.2013 17:37, Petr Viktorin wrote:
On 02/19/2013 01:51 PM, Jan Cholasta wrote:
Hi,
On 5.2.2013 18:02, Petr Viktorin wrote:
CIDict, our case-insensitive dictionary, inherits from dict but did
not
reimplement the full dict interface. Calling the missing methods
silently invoked case-sensitive behavior. Our code seems to avoid
that,
but it's a bit of a minefield for new development.
Patch 119 adds the missing dict methods (except
view{items,keys,values}(), which now raise errors), and adds tests.
Can you please also add the (obj, **kwargs) and (**kwargs)
variants of
__init__ and update?
Added, thanks for the catch.
Patches 117-118 modernize the testsuite a bit (these have been
sitting
in my queue for a while, I think now is a good time to submit them):
The first one moves some old tests from the main code tree to
tests/.
(The adtrust_install test wasn't run before, this move makes nose
notice
it).
The second converts CIDict's unittest-based suite to nose.
Honza
Whoa, I totally forgot about these patches!
Can you please rebase them?
Sure!
One more comment:
Document that CIDict.copy() returns a plain dict.
Why does it return a plain dict? I think it should return a CIDict,
otherwise it is not actually a copy, right?
I wanted to keep the existing (intended) functionality.
But since we don't actually use CIDict.copy() anywhere any more, I've
made the change.
Thanks.
P.S. I recently came across a bug in python-ldap where something like
CIDict({'cn': ['name1', 'name2'], 'cN': ['name3']}) will throw away some
of the values.
This is expected at the CIDict level, but if you're working with dicts
of lists it's something to keep an eye out for.
This is a good point. IIRC when you use such a dict in python-ldap, it
will fail with TYPE_OR_VALUE_EXISTS. I think we should raise an error in
CIDict as well if such a dict is used in __init__() and update(), as
this kind of error is very hard to track otherwise.
Honza
Right. Here's a patch that does that.
Thanks.
ACK to all four patches.
Honza
--
Jan Cholasta
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
