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

Ordered dicts #12

Closed
rasmusagren opened this issue May 4, 2018 · 12 comments
Closed

Ordered dicts #12

rasmusagren opened this issue May 4, 2018 · 12 comments
Assignees

Comments

@rasmusagren
Copy link

For Python>=3.6 normal dicts are now ordered as well. Could you please add so that their order is maintained as well? Thanks!

@jonas-eschle
Copy link
Contributor

Yes, but Python >= 3.7, as for 3.6, the ordering is considered a "CPython implementation detail that you should not rely on". Do you agree?

Further, the idea is to only allow that for those python versions and raise an error for lower versions (you have of course always the possibility of calling it with OrderedDict(dict) instead of dict).

@SebastianJL: what's your opinion on that?

@rasmusagren
Copy link
Author

True, but you might as well output it in the order given by the dict. If it's ordered everything is fine, if it's not then no problem either. Just skip the explicit re-ordering.

@jonas-eschle
Copy link
Contributor

Okay, but I cannot really see a (non-harmful) use-case for that. It will be implemented anyway to support the 3.7+ dicts, so technically it is not a problem to add, but for <=3.6 I can only think of two cases:

  1. You care about the order in python. Then you use OrderedDict.
  2. You don't care about the order and therefore use dict. Then why should it be preserved during dumping and loading?

Can you think of a use-case where someone would use dict (<=3.6) and care about the dumping order?

@rasmusagren
Copy link
Author

I agree that it would be a bad idea to develop software that relies on that dicts are ordered in python 3.6, but here I'm using a third-party module for one-time data analysis. Dicts don't have to be ordered to be used internally in that tool, but it happens to be convenient for me that they are. That way I can read a yaml-file, have the tool modify the contents and then dump it back without reordering. It just seems much more intrusive to explicitly choose to sort entries alphabetically rather than just using the ordering that is native to whatever python version you are running.

@jonas-eschle
Copy link
Contributor

I see! I would propose you to use the loader/dumber as following:

foo.bar.dump(OrderedDict(your_dict))
foo.bar.load(dict(your_dict.yaml))

with your_dict being the dictionary that you use and your_dict.yaml the file where you dumped it to.

Not a lot of extra-code, but works!

@rasmusagren
Copy link
Author

That's what I do now, but since it's not my code that does the dumping I have to patch the other tool. A little cumbersome but fine.

I guess it's this section in representer.py in pyyaml that is the culprit, so maybe I should whine there instead :) However, since the purpose of your tool is to fix some stuff that pyyaml should have implemented if they were a bit faster with releases, I still think it fits with your tool.

mapping = list(mapping.items())
try:
    mapping = sorted(mapping)
except TypeError:
    pass

@jonas-eschle
Copy link
Contributor

Alright, unfortunately I seem still to miss one point: if the dumping happens in the other tool, does it use yamlloader inside? Because if it does not, I don't see how changing anything inside yamlloader changes this behavior. What do I miss?
(And if it uses yamlloader already, then I think it should pass OrderedDicts already, right? Or does it use yamlloader with dict?)

Anyway, I would recommend you then to change the other code. It's a long-term investment and you're on the save side (being in the field of data-analysis myself, this is what I usually end up with doing).

@rasmusagren
Copy link
Author

I understand the confusion :) The other tools uses yamlloader, but the structure being dumped contains a lot of stuff, some of which are OrderedDicts and some which are dicts. I happen to need one of the dicts to be ordered. I see that there is a PR to pyyaml which will fix it there (yaml/pyyaml#143), and they say that they will release a new version soon so I guess that's the appropriate fix.

@jonas-eschle
Copy link
Contributor

jonas-eschle commented May 7, 2018

Ah, got it! Yes, so I would go with the upstream fix in pyyaml. If that's fixed, it will work with yamlloader as well of course.

If they don't do that (for whatever reason), then we'll may implement that here.

Closing for now, feel free to reopen if it does not get fixed in pyyaml!

@jonas-eschle jonas-eschle self-assigned this May 21, 2018
@perlpunk
Copy link

perlpunk commented Jul 1, 2018

the yaml/pyyaml#143 PR merge will probably take a bit longer. we had some problems with releasing a new version, and some api changes we need to make for the next version touch the same code as my PR

@jonas-eschle jonas-eschle reopened this Jul 1, 2018
@jonas-eschle jonas-eschle added this to the python 3.7 milestone Jul 1, 2018
@jonas-eschle
Copy link
Contributor

@perlpunk thanks for letting us know!

@rasmusagren We will soon release the 3.7 compatible version, which keeps the order with normal dicts as well.
You can simply switch a flag inside yamlloader and basically tell that you are using Python 3.7 or above and the dicts will preserve the order (this, of course, is a hack and fails for python <=3.5). But this should work for your case then.

How does that sound to you?

@jonas-eschle
Copy link
Contributor

Finally also updated conda version, sorry for the delay!

@jonas-eschle jonas-eschle removed this from the python 3.7 milestone Aug 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants