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

allow ordered dict export to yaml #198

Merged
merged 5 commits into from
Jun 7, 2017
Merged

Conversation

bridwell
Copy link
Contributor

Currently, in utils.yamlio, dictionaries exported via convert_to_yaml use a hard-coded list to control the ordering of keys in the output file. This has two drawbacks (1) the list has to be updated when new parameters are added and (2) only the outer dictionaries ordering can be controlled, the ordering of inner dictionaries will not follow the order list.

This PR adds support for exporting ordered dictionaries to yaml in utils.yamlio. For example, the following:

inner_dict = OrderedDict()
inner_dict['z'] = 'had'
inner_dict['a'] = 'a'
inner_dict['f'] = 'little'

outer_dict = OrderedDict()
outer_dict[10] = 'marry'
outer_dict['inner'] = inner_dict
outer_dict['a'] = 'lamb'

yamlio.convert_to_yaml(outer_dict, my_file.yaml)

Will have the following yaml:

10: marry

inner:
    z: had
    a: a
    f: little

a: lamb

Note that if a ordered dictionary is provided (as in the current example), its ordering will override the hard-code ordering the yamlio file.

@coveralls
Copy link

coveralls commented May 18, 2017

Coverage Status

Coverage increased (+0.04%) to 94.327% when pulling ba03b34 on AZMAG:odict_to_yaml into a14b287 on UDST:master.

@pksohn pksohn self-requested a review May 22, 2017 19:02
@pksohn
Copy link
Contributor

pksohn commented May 22, 2017

Great idea Scott - I'll review this later today

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 94.129% when pulling aa9406f on AZMAG:odict_to_yaml into a14b287 on UDST:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.3%) to 94.036% when pulling 542f926 on AZMAG:odict_to_yaml into a14b287 on UDST:master.

@coveralls
Copy link

coveralls commented May 22, 2017

Coverage Status

Coverage increased (+0.04%) to 94.334% when pulling 058a06f on AZMAG:odict_to_yaml into a14b287 on UDST:master.

Copy link
Contributor

@pksohn pksohn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good other than a very minor typo. I suppose this opens us up to moving the DCM and regression YAML methods to OrderedDicts instead of relying on the list in this module. Are you planning to work on that too, or just using this for something external?

Fun sidenote: Python 3.6 now has order preserved in default dictionaries, which probably won't be fully relevant for us for many years, but good to be aware of.

@@ -146,3 +147,53 @@ def test_frame_to_yaml_safe():
'col2': {0: 'a', 1: 'b', 2: 'c'}}
y = yaml.dump(d, default_flow_style=False)
assert_dfs_equal(pd.DataFrame(yaml.load(y)), df)


def test_orderered_dict():
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a minor typo here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was planning on using this for DCM updates. It's really just cosmetic, but I wanted a little more control over the look of the yaml.

One thing to note is that constructing a pandas DataFrame with an ordered dict will allow for controlling the column order, but not necessarily the row/index order. However, I think in most cases this can be handled with some re-indexing from the dictionary keys:

 pd.DataFrame.from_dict(my_odict, orient='index').reindex(my_odict.keys()).T

@coveralls
Copy link

coveralls commented May 23, 2017

Coverage Status

Coverage increased (+0.04%) to 94.334% when pulling 9abd5f7 on AZMAG:odict_to_yaml into a14b287 on UDST:master.

@pksohn pksohn merged commit 1982464 into UDST:master Jun 7, 2017
@bridwell bridwell deleted the odict_to_yaml branch June 12, 2017 16:26
@smmaurer smmaurer mentioned this pull request Sep 10, 2019
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants