classification
Title: allow argparse FileType to accept encoding and errors arguments
Type: enhancement Stage: committed/rejected
Components: Library (Lib) Versions: Python 3.4
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: petri.lehtinen Nosy List: amaury.forgeotdarc, bethard, chris.jerdonek, eric.araujo, ezio.melotti, lum, petri.lehtinen, python-dev
Priority: low Keywords: easy, needs review, patch

Created on 2011-02-10 15:47 by bethard, last changed 2012-12-17 07:41 by lum. This issue is now closed.

Files
File name Uploaded Description Edit
filetype11175.patch lum, 2012-10-23 12:06 patch review
filetype11175.patch lum, 2012-10-26 15:11 review
filetype11175.patch lum, 2012-11-27 15:15 review
filetype11175.patch lum, 2012-12-03 07:43 review
Messages (18)
msg128301 - (view) Author: Steven Bethard (bethard) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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!
History
Date User Action Args
2012-12-17 07:41:25lumsetmessages: + msg177638
2012-12-15 20:46:16petri.lehtinensetstatus: open -> closed
resolution: fixed
messages: + msg177562

stage: commit review -> committed/rejected
2012-12-15 20:44:02python-devsetnosy: + python-dev
messages: + msg177561
2012-12-03 07:43:05lumsetfiles: + filetype11175.patch

messages: + msg176829
2012-11-27 15:15:34lumsetfiles: + filetype11175.patch

messages: + msg176479
2012-11-27 12:21:30lumsetmessages: + msg176472
2012-11-27 06:42:09petri.lehtinensetmessages: + msg176464
2012-11-26 14:12:13chris.jerdoneksetnosy: + chris.jerdonek
messages: + msg176414
2012-11-26 09:00:36petri.lehtinensetassignee: petri.lehtinen
messages: + msg176404
2012-11-26 08:18:22lumsetmessages: + msg176403
2012-10-26 15:28:46berker.peksagsetnosy: - berker.peksag
2012-10-26 15:15:30petri.lehtinensetkeywords: + needs review

messages: + msg173852
stage: patch review -> commit review
2012-10-26 15:11:45lumsetfiles: + filetype11175.patch

messages: + msg173851
2012-10-24 21:25:27ezio.melottisetnosy: + ezio.melotti
2012-10-23 13:11:50lumsetmessages: + msg173611
2012-10-23 12:21:48petri.lehtinensetnosy: + petri.lehtinen

messages: + msg173603
stage: needs patch -> patch review
2012-10-23 12:06:15lumsetfiles: + filetype11175.patch

nosy: + lum
messages: + msg173599

keywords: + patch
2012-10-18 16:04:58petri.lehtinensetversions: + Python 3.4, - Python 3.3
2011-12-12 16:44:56berker.peksagsetnosy: + berker.peksag
2011-07-19 14:56:09eric.araujosetkeywords: + easy
nosy: + eric.araujo
2011-02-10 17:35:23bethardsetmessages: + msg128318
2011-02-10 17:02:12amaury.forgeotdarcsetnosy: + amaury.forgeotdarc
messages: + msg128310
2011-02-10 15:47:13bethardcreate