Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

allow argparse FileType to accept encoding and errors arguments #55384

Closed
bethard mannequin opened this issue Feb 10, 2011 · 18 comments
Closed

allow argparse FileType to accept encoding and errors arguments #55384

bethard mannequin opened this issue Feb 10, 2011 · 18 comments
Assignees
@akheron
Labels
easy stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@bethard
Copy link
Mannequin

bethard mannequin commented Feb 10, 2011

BPO 11175
Nosy @amauryfa, @ezio-melotti, @merwok, @cjerdonek, @akheron
Files
  • filetype11175.patch: patch
  • filetype11175.patch
  • filetype11175.patch
  • filetype11175.patch
    		•

    Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/akheron'
closed_at = <Date 2012-12-15.20:46:16.690>
created_at = <Date 2011-02-10.15:47:13.087>
labels = ['easy', 'type-feature', 'library']
title = 'allow argparse FileType to accept encoding and errors arguments'
updated_at = <Date 2012-12-17.07:41:25.024>
user = 'https://bugs.python.org/bethard'

    bugs.python.org fields:

    activity = <Date 2012-12-17.07:41:25.024>
actor = 'lum'
assignee = 'petri.lehtinen'
closed = True
closed_date = <Date 2012-12-15.20:46:16.690>
closer = 'petri.lehtinen'
components = ['Library (Lib)']
creation = <Date 2011-02-10.15:47:13.087>
creator = 'bethard'
dependencies = []
files = ['27674', '27729', '28139', '28195']
hgrepos = []
issue_num = 11175
keywords = ['patch', 'easy', 'needs review']
message_count = 18.0
messages = ['128301', '128310', '128318', '173599', '173603', '173611', '173851', '173852', '176403', '176404', '176414', '176464', '176472', '176479', '176829', '177561', '177562', '177638']
nosy_count = 8.0
nosy_names = ['amaury.forgeotdarc', 'bethard', 'ezio.melotti', 'eric.araujo', 'chris.jerdonek', 'python-dev', 'petri.lehtinen', 'lum']
pr_nums = []
priority = 'low'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue11175'
versions = ['Python 3.4']
    @bethard
    Copy link
    Mannequin Author

    bethard mannequin commented Feb 10, 2011

    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().

    @bethard bethard mannequin added stdlib Python modules in the Lib dir type-feature A feature request or enhancement labels Feb 10, 2011
    @amauryfa
    Copy link
    Member

    The encoding and errors arguments are probably useful, but why would you use the codecs module?

    @bethard
    Copy link
    Mannequin Author

    bethard mannequin commented Feb 10, 2011

    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.

    @merwok merwok added the easy label Jul 19, 2011
    @lum
    Copy link
    Mannequin

    lum mannequin commented Oct 23, 2012

    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...

    @akheron
    Copy link
    Member

    akheron commented Oct 23, 2012

    Lucas: You only added tests for the repr, you should probably test the actual new functionality, too.

    @lum
    Copy link
    Mannequin

    lum mannequin commented Oct 23, 2012

    OK, as discussed offline with Petri I'll put some tests to ensure that open() is called the right way (using unittest.mock.patch).

    @lum
    Copy link
    Mannequin

    lum mannequin commented Oct 26, 2012

    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.

    @akheron
    Copy link
    Member

    akheron commented Oct 26, 2012

    LGTM. Let's wait for Steven's comments as he's the maintainer of argparse.

    @lum
    Copy link
    Mannequin

    lum mannequin commented Nov 26, 2012

    Is there something I can do something to move this forward?

    @akheron
    Copy link
    Member

    akheron commented Nov 26, 2012

    Since Steven is not responding, I think I can commit it at some point. Thanks for the remainder :)

    @akheron akheron self-assigned this Nov 26, 2012
    @cjerdonek
    Copy link
    Member

    The patch needs documentation, though, right?

    http://docs.python.org/dev/library/argparse.html#filetype-objects

    @akheron
    Copy link
    Member

    akheron commented Nov 27, 2012

    Right, good point. Lucas, could you fix the documentation too?

    @lum
    Copy link
    Mannequin

    lum mannequin commented Nov 27, 2012

    OK, I'll give it a try.

    @lum
    Copy link
    Mannequin

    lum mannequin commented Nov 27, 2012

    Added some documentation for the patch. Let me know what you think.

    @lum
    Copy link
    Mannequin

    lum mannequin commented Dec 3, 2012

    As per Ezio's comment, changed "l1" to "utf-8" in the example of the doc.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Dec 15, 2012

    New changeset d5a0698a8354 by Petri Lehtinen in branch 'default':
    bpo-11175: argparse.FileType now accepts encoding and errors arguments.
    http://hg.python.org/cpython/rev/d5a0698a8354

    @akheron
    Copy link
    Member

    akheron commented Dec 15, 2012

    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

    @akheron akheron closed this as completed Dec 15, 2012
    @lum
    Copy link
    Mannequin

    lum mannequin commented Dec 17, 2012

    Sorry for the little glitches you had to fix, I wonder why I didn't catch them.
    Anyways, thanks Petri!

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees

    @akheron akheron

    Labels
    easy stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    4 participants
    @akheron @cjerdonek @merwok @amauryfa