I've applied those patches. Fixed in git for 1.1.2, unless you spot any further problems.
(In reply to comment #11)
> * I'm not sure that you need the lambda in
> test_dbus_exception_convert_str_fail().
You do. In the older form of assertRaises, the thing that raises the exception needs to be a callable, so that the exception isn't already raised before it has a chance to wrap it in a "try:".
This is much nicer in the newer form, but I don't think that's a good enough reason to require Python 2.7.
> * Can you provide some more detail about the weird up-chaining you're seeing
> with Exception.__unicode__?
It's really BaseException.__unicode__, which is this pseudocode:
if self.__str__ isn't BaseException.__str__:
return unicode(self.__str__()) # <---------
if len(self.args) == 0:
return ""
if len(self.args) == 1:
return unicode(self.args[0])
# len(self.args) > 1
return unicode(self.args)
The __str__ call will raise an exception if the message is non-ASCII but the default codec is ASCII, so we lose.
My implementation of __unicode__ is a slightly more concise version of the rest of that pseudocode - I'm essentially emulating what would happen if we didn't have a __str__ method at all. We need __str__ for Python 3, AIUI.
> * I'd probably swap this code around (I always dislike testing a negative
> when testing a positive will do just as well):
>
> + if self._dbus_error_name is not None:
> + return '%s: %s' % (self._dbus_error_name, s)
> + else:
> + return s
It was already like that, and I tend to think of "is not None" as the "positive" case - conceptually, I'm saying "if there is a _dbus_error_name". I'd accept a patch to change both copies if you feel strongly about this, though.
I've applied those patches. Fixed in git for 1.1.2, unless you spot any further problems.
(In reply to comment #11) exception_ convert_ str_fail( ).
> * I'm not sure that you need the lambda in
> test_dbus_
You do. In the older form of assertRaises, the thing that raises the exception needs to be a callable, so that the exception isn't already raised before it has a chance to wrap it in a "try:".
This is much nicer in the newer form, but I don't think that's a good enough reason to require Python 2.7.
> * Can you provide some more detail about the weird up-chaining you're seeing __unicode_ _?
> with Exception.
It's really BaseException. __unicode_ _, which is this pseudocode:
if self.__str__ isn't BaseException. __str__ : self.__ str__() ) # <---------
return unicode(
if len(self.args) == 0:
return ""
if len(self.args) == 1: self.args[ 0])
return unicode(
# len(self.args) > 1
return unicode(self.args)
The __str__ call will raise an exception if the message is non-ASCII but the default codec is ASCII, so we lose.
My implementation of __unicode__ is a slightly more concise version of the rest of that pseudocode - I'm essentially emulating what would happen if we didn't have a __str__ method at all. We need __str__ for Python 3, AIUI.
> * I'd probably swap this code around (I always dislike testing a negative error_name is not None: dbus_error_ name, s)
> when testing a positive will do just as well):
>
> + if self._dbus_
> + return '%s: %s' % (self._
> + else:
> + return s
It was already like that, and I tend to think of "is not None" as the "positive" case - conceptually, I'm saying "if there is a _dbus_error_name". I'd accept a patch to change both copies if you feel strongly about this, though.