Add preserve_order option to toggle between using an OrderedDict or normal dict #127

wants to merge 2 commits into


None yet

2 participants

bruth commented Nov 14, 2013

This addresses #96 by giving the option of preserving the order of INFO fields.

bruth added some commits Mar 6, 2013
@bruth bruth Add preserve_order option
This enables toggling between an OrderedDict vs. a normal dict
@bruth bruth Change preserve_order to default to True for backwards compat
Amend the docstring to describe what the parameter does.
@martijnvermaat martijnvermaat commented on the diff Nov 14, 2013
@@ -350,7 +358,7 @@ def _parse_info(self, info_str):
return {}
entries = info_str.split(';')
- retdict = OrderedDict()
martijnvermaat Nov 14, 2013 Collaborator

This is the only line that's really hitting performance, right?


I'm not too happy with this approach. I think we really need the order in the header lines and it doesn't make sense to make that configurable.

The line I annotated is the thing you're really looking for and I do think it makes sense to change that to an ordinary dictionary while keeping the ordered dictionary for the header lines. We wouldn't need any configuration that way.

(That one line was also added later, following #46.)

I'll continue the discussion in #96 now, where we have some more context.

bruth commented Nov 14, 2013

@martijnvermaat I would thrilled to have that single line change. I made it configurable merely so the behavior did not change for others using the library.

@bruth bruth closed this Nov 19, 2013

Alternative solution in #128 was merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment