| msg132309 - (view) |
Author: Filip Gruszczyński (gruszczy) |
Date: 2011-03-27 11:12 |
| xdrlib defines ConversionError, but very seldom uses it. For example: def pack_float(self, x): try: self.__buf.write(struct.pack('>f', x)) except struct.error as msg: raise ConversionError(msg) But it doesn't do so here: def pack_uint(self, x): self.__buf.write(struct.pack('>L', x)) Shouldn't that be more consistent? I am happy to write a patch, that will make xdrlib raise ConversionError, as well as write proper test (I believe xdrlib tests should get some love altogether, so I would add a separate test case for this). |
|
|
| msg138783 - (view) |
Author: Petri Lehtinen (petri.lehtinen) *  |
Date: 2011-06-21 12:30 |
| This seems like a bug worth fixing. The ConversionError exception has been documented, an there's an example in the docs that suggest that at least all packing fails with a ConversionError. |
|
|
| msg144180 - (view) |
Author: Filip Gruszczyński (gruszczy) |
Date: 2011-09-17 10:59 |
| Patch with tests. |
|
|
| msg151291 - (view) |
Author: Filip Gruszczyński (gruszczy) |
Date: 2012-01-15 15:50 |
| Bump! It's almost 3 months since I posted the patch, so I would like to remind about this bug. |
|
|
| msg151293 - (view) |
Author: Georg Brandl (georg.brandl) * |
Date: 2012-01-15 16:54 |
| Looks good to me. |
|
|
| msg162367 - (view) |
Author: Petri Lehtinen (petri.lehtinen) * |
Date: 2012-06-05 19:24 |
| I see one obvious issue with the patch: The ConversionErrors it creates are passed the struct.error or TypeError instance as a parameter. The first argument of these exceptions would be better, i.e. try: ... except struct.error as e: raise ConversionError(e.args[0]) Furthermore, my ear thinks that raises_conversion_error would be a better name for the decorator than raising_conversion_error. Anyway, I think the whole ConversionError is a bit problematic, as either TypeError or ValueError would be the most appropriate exception, depending on the situation. For example: p = Packer() p.pack_int('foo') # should raise a TypeError p.pack_int(2**100) # should raise a ValueError This would be slightly harder to implement, though, as struct.error has exactly the same problem. |
|
|
| msg228893 - (view) |
Author: PCManticore (Claudiu.Popa) *  |
Date: 2014-10-09 19:16 |
| Here's a refreshed patch: - raising_conversion_error is now raise_conversion_error - the decorator uses functools.wraps - the ConversionError is instantiated with the first argument of the caught expression - the reraising of ConversionError has the exception context supressed. |
|
|
| msg228951 - (view) |
Author: Petri Lehtinen (petri.lehtinen) * |
Date: 2014-10-10 05:23 |
| LGTM |
|
|
| msg229020 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2014-10-10 18:30 |
| New changeset 9cee201388c9 by Petri Lehtinen in branch '2.7': Issue #11694: Raise ConversionError in xdrlib as documented https://hg.python.org/cpython/rev/9cee201388c9 New changeset 7ef6e5f53418 by Petri Lehtinen in branch '3.4': Issue #11694: Raise ConversionError in xdrlib as documented https://hg.python.org/cpython/rev/7ef6e5f53418 New changeset 8e3387f566f6 by Petri Lehtinen in branch 'default': #11694: merge with 3.4 https://hg.python.org/cpython/rev/8e3387f566f6 |
|
|
| msg229021 - (view) |
Author: Petri Lehtinen (petri.lehtinen) * |
Date: 2014-10-10 18:33 |
| Applied, thanks! |
|
|