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

Order CSV header fields #72029

Closed
holdenweb opened this issue Aug 23, 2016 · 11 comments
Closed

Order CSV header fields #72029

holdenweb opened this issue Aug 23, 2016 · 11 comments
Assignees
@rhettinger
Labels
stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@holdenweb
Copy link
Member

BPO 27842
Nosy @rhettinger, @bitdancer
Files
  • csv.patch
  • csv_full.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/rhettinger'
closed_at = <Date 2016-08-30.19:37:11.878>
created_at = <Date 2016-08-23.16:42:42.354>
labels = ['type-feature', 'library']
title = 'Order CSV header fields'
updated_at = <Date 2016-08-30.20:47:27.723>
user = 'https://bugs.python.org/holdenweb'

    bugs.python.org fields:

    activity = <Date 2016-08-30.20:47:27.723>
actor = 'holdenweb'
assignee = 'rhettinger'
closed = True
closed_date = <Date 2016-08-30.19:37:11.878>
closer = 'rhettinger'
components = ['Library (Lib)']
creation = <Date 2016-08-23.16:42:42.354>
creator = 'holdenweb'
dependencies = []
files = ['44203', '44235']
hgrepos = []
issue_num = 27842
keywords = ['patch', 'needs review']
message_count = 11.0
messages = ['273486', '273487', '273489', '273530', '273540', '273694', '273695', '273709', '273956', '273957', '273963']
nosy_count = 4.0
nosy_names = ['rhettinger', 'holdenweb', 'r.david.murray', 'python-dev']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'patch review'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue27842'
versions = ['Python 3.6']
    @holdenweb
    Copy link
    Member Author

    It's sometimes annoying that a csv.DictReader doesn't retain the field ordering given in the first line of the file. Sometimes it matters.

    This patch converts the reader so that it returns an OrderedDict rather than a plain dict, thereby retaining the ordering.

    All tests still pass (though I haven't yet added a test to verify that the field ordering *is* retained - didn't think it was worth it if the patch won't be added, but will happily add that test otherwise).

    I have updated the documentation, but was unable in the time available to find out how to correctly reference the OrderedDict class so that it was correctly hyperlinked.

    @holdenweb holdenweb added the stdlib Python modules in the Lib dir label Aug 23, 2016
    @holdenweb
    Copy link
    Member Author

    Sorry, deleted the originally submitted (incorrect) patch file.

    @bitdancer
    Copy link
    Member

    I think this seems reasonable, now that OrderedDict is in C.

    @bitdancer bitdancer added the type-feature A feature request or enhancement label Aug 23, 2016
    @rhettinger
    Copy link
    Contributor

    This looks like a nice improvement. The patch needs a test and the docs need a versionchanged entry.

    @rhettinger rhettinger self-assigned this Aug 24, 2016
    @holdenweb
    Copy link
    Member Author

    Testing could be interesting. I'm thinking of generating five random string keys with a couple of rows of data, creating csv StringIOs (using pure Python) for all 120 combinations and verifying that they read back in the order they were written.

    We should also test that OrderedDicts write correctly with a DictWriter and the same key sets.

    It's a fairly haphazard test plan, so I'll be happy to hear more thorough suggestions. We could at a pinch reduce the number of keys to three if speed considerations dictate.

    BTW, what happened to NEWS.txt? :)

    @rhettinger
    Copy link
    Contributor

    Consider using itertools.permutations() to generate the 120 cases cases.

    The news entry goes into Misc/NEWS (there is not .txt extension).

    @holdenweb
    Copy link
    Member Author

    Is there another way? :)

    Sent from my iPhone

    On 26 Aug 2016, at 12:16, Raymond Hettinger <report@bugs.python.org> wrote:

    Raymond Hettinger added the comment:

    Consider using itertools.permutations() to generate the 120 cases cases.

    The news entry goes into Misc/NEWS (there is not .txt extension).

    ----------

    Python tracker <report@bugs.python.org>
    <https://bugs.python.org/issue27842\>

    @holdenweb
    Copy link
    Member Author

    OK, here's what I think should be close to the final patch. I've updated the documentation, rebuilt it and verified it reads OK, and confirmed that the new code passes all tests except those skipped for platform reasons (I think they expect a Windows environment). This includes the new test to confirm that ordering is retained over all 120 possible combinations of five keys.

    Please let me know if any further updates are needed.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Aug 30, 2016

    New changeset bb3e2a5be31b by Raymond Hettinger in branch 'default':
    Issue bpo-27842: The csv.DictReader now returns rows of type OrderedDict.
    https://hg.python.org/cpython/rev/bb3e2a5be31b

    @rhettinger
    Copy link
    Contributor

    Thanks Steve.

    @holdenweb
    Copy link
    Member Author

    A pleasure. Pretty heavily committed at present, but all Python related so maybe there'll be more small positive improvements.

    @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

    @rhettinger rhettinger

    Labels
    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
    3 participants
    @holdenweb @bitdancer @rhettinger