homepage

> To be clear, the errback change and the unpickleable result
> change are actually orthogonal, right?
Yes, it could be a separate issue. Jesse, do you think I should I open
up a separate issue for this?
> Why not add an error_callback for map_async as well?
That's a good idea!
> Any reason you chose to use a different internal name
> (errback versus error_callback)? It seems cleaner to me
> to be consistent about the name.
It was actually a mistake. The argument was ``errback`` before, so
it's just a leftover from the previous name.
> In general, I'm wary of nonessential whitespace changes...
> did you mean to include these?
Of course not.
> Using "assertTrue" seems misleading. "assertIsNotNone" is what
> really mean, right? Although, I believe that's redundant,
> since presumably self.assertIsInstance(None, KeyError) will
> error out anyway (I haven't verified this).
bool(KeyError("foo")) is True and bool(None) is False, so it works either way. It could theoretically result in a false negative if
the exception class tested overrides __nonzero__, but that is unlikely
to happen as the target always returns KeyError anyway (and the test below ensures it) It's just a habit of mine, unless I really want to test for Noneness, I just use assertTrue, but I'm not against changing it to assertIsNotNone either.
> Under what circumstances would these be None? (Perhaps you
> want wrapped.exc != 'None'?) The initializer for
> MaybeEncodingError enforces the invariant that exc/value are strings
> right?
It's just to test that these are actually set to something.
Even an empty string passes with assertIsNone instead of assertTrue.
Maybe it's better to test the values set, but I didn't bother.