msg128301 - (view) |
Author: Steven Bethard (bethard) *  |
Date: 2011-02-10 15:47 |
Suggestion from a personal email: Allow FileType to accept encoding and errors arguments and pass these as keyword arguments to codecs.open() instead of open(). |
|
|
msg128310 - (view) |
Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) *  |
Date: 2011-02-10 17:02 |
The encoding and errors arguments are probably useful, but why would you use the codecs module? |
|
|
msg128318 - (view) |
Author: Steven Bethard (bethard) *  |
Date: 2011-02-10 17:35 |
Probably because the suggestion came from someone thinking about both Python 2 and 3. But given that this feature request can only target Python 3.3, you're absolutely right that there's no need to go through the codecs module. |
|
|
msg173599 - (view) |
Author: Lucas Maystre (lum) * |
Date: 2012-10-23 12:06 |
Here's an attempt at implementing this (my first contribution). Some notes: - I tried to keep `__repr__()` backwards compatible. It would have been easier to inherit from `_AttributeHolder`, but maybe this might break some things... - I added some test cases for `__repr__()`, but that's it. I don't think there's any other sensible test to do, as we're really just passing stuff to `open()` - not sure about the style, especially line breaks... |
|
|
msg173603 - (view) |
Author: Petri Lehtinen (petri.lehtinen) *  |
Date: 2012-10-23 12:21 |
Lucas: You only added tests for the repr, you should probably test the actual new functionality, too. |
|
|
msg173611 - (view) |
Author: Lucas Maystre (lum) * |
Date: 2012-10-23 13:11 |
OK, as discussed offline with Petri I'll put some tests to ensure that open() is called the right way (using unittest.mock.patch). |
|
|
msg173851 - (view) |
Author: Lucas Maystre (lum) * |
Date: 2012-10-26 15:11 |
Alright, here's a version with more tests (unittest.mock is awesome!). I think it tests exactly what it should (and no more), i.e. that the arguments are correctly passed to `open`. |
|
|
msg173852 - (view) |
Author: Petri Lehtinen (petri.lehtinen) *  |
Date: 2012-10-26 15:15 |
LGTM. Let's wait for Steven's comments as he's the maintainer of argparse. |
|
|
msg176403 - (view) |
Author: Lucas Maystre (lum) * |
Date: 2012-11-26 08:18 |
Is there something I can do something to move this forward? |
|
|
msg176404 - (view) |
Author: Petri Lehtinen (petri.lehtinen) *  |
Date: 2012-11-26 09:00 |
Since Steven is not responding, I think I can commit it at some point. Thanks for the remainder :) |
|
|
msg176414 - (view) |
Author: Chris Jerdonek (chris.jerdonek) *  |
Date: 2012-11-26 14:12 |
The patch needs documentation, though, right? http://docs.python.org/dev/library/argparse.html#filetype-objects |
|
|
msg176464 - (view) |
Author: Petri Lehtinen (petri.lehtinen) *  |
Date: 2012-11-27 06:42 |
Right, good point. Lucas, could you fix the documentation too? |
|
|
msg176472 - (view) |
Author: Lucas Maystre (lum) * |
Date: 2012-11-27 12:21 |
OK, I'll give it a try. |
|
|
msg176479 - (view) |
Author: Lucas Maystre (lum) * |
Date: 2012-11-27 15:15 |
Added some documentation for the patch. Let me know what you think. |
|
|
msg176829 - (view) |
Author: Lucas Maystre (lum) * |
Date: 2012-12-03 07:43 |
As per Ezio's comment, changed "l1" to "utf-8" in the example of the doc. |
|
|
msg177561 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2012-12-15 20:44 |
New changeset d5a0698a8354 by Petri Lehtinen in branch 'default': #11175: argparse.FileType now accepts encoding and errors arguments. http://hg.python.org/cpython/rev/d5a0698a8354 |
|
|
msg177562 - (view) |
Author: Petri Lehtinen (petri.lehtinen) *  |
Date: 2012-12-15 20:46 |
Committed, thanks for the patch! I made little tweaks before committing: - changed l1 to UTF-8 in the example (guess you mis-uploaded the latest version) - changed the argument to 'replace' in test_r_1_replace - fixed 3-space indentation to 4-space indentation in test_open_args |
|
|
msg177638 - (view) |
Author: Lucas Maystre (lum) * |
Date: 2012-12-17 07:41 |
Sorry for the little glitches you had to fix, I wonder why I didn't catch them. Anyways, thanks Petri! |
|
|