Skip to content

Allow to turn off sorting keys in Dumper#143

Closed
perlpunk wants to merge 1 commit intoyaml:masterfrom
perlpunk:dumper-sortkeys
Closed

Allow to turn off sorting keys in Dumper#143
perlpunk wants to merge 1 commit intoyaml:masterfrom
perlpunk:dumper-sortkeys

Conversation

@perlpunk
Copy link
Copy Markdown
Member

@perlpunk perlpunk commented Mar 16, 2018

See issue #110

This allows to do:

output = yaml.dump(data, sort_keys=False)

The default is True because of backwards compatibility.

This can be useful for python >= 3.7 to preserve key order. Also not sorting will probably be faster.

I didn't add a test because I'm not yet familiar with testing in python
and couldn't find out how the pyyaml test suite works.

Also i don't know where to document that.

@perlpunk
Copy link
Copy Markdown
Member Author

Travis tests are failing. I need to find out how to run the tests locally. Any hints for a python beginner?

@perlpunk
Copy link
Copy Markdown
Member Author

thanks @ingydotnet for python setup.py test
I pushed a fix for cyaml.py, tests are passing now for python > 2.6

@perlpunk
Copy link
Copy Markdown
Member Author

I started to write a test for this. Will probably push it tomorrow

@perlpunk
Copy link
Copy Markdown
Member Author

rebased to master, tests now including 3.7-dev, and 2.6 is passing

@perlpunk
Copy link
Copy Markdown
Member Author

perlpunk commented Jul 1, 2018

Rebased to current master, squashed commits.
Tests are passing (and now include the libyaml tests also)

@dsposito
Copy link
Copy Markdown

dsposito commented Jul 6, 2018

This is great! Looking forward to this being merged! 🚀

@perlpunk perlpunk removed the request for review from sigmavirus24 July 6, 2018 16:16
@jasonrhaas
Copy link
Copy Markdown

Any progress on this PR?

@dsposito
Copy link
Copy Markdown

dsposito commented Aug 6, 2018

@jasonrhaas I'm hoping this eventually gets merged to source but for now oyaml does the trick.

@Ark-kun
Copy link
Copy Markdown

Ark-kun commented Oct 3, 2018

Please, merge this. I've spent countless hours trying to work around this.
P.S. No, oyaml is not a solution for us, since our team is already using pyyaml and does not want to change all code files.
P.P.S. No, OrderedDict is not a solution, because it radically changes the output and yaml types. (mapping to sequence)

@dilumr
Copy link
Copy Markdown

dilumr commented Oct 24, 2018

I too am interested is this. Would like to use in a feature where a tool slightly modifies YAML files that are checked into source. Would love the edits to look slight. The YAML files are initially ordered by hand, to expecting keys to be alphanumerically sorted is unrealistic; humans tend to group keys by purpose and/or importance.

@gatopeich
Copy link
Copy Markdown

gatopeich commented Nov 20, 2018

Dear Perl punk (since you seem the only maintainer left here), leaving "sorted" by default is still wrong.

Sorting the keys is a superfluous operation wrt the YAML spec. It is also anti-pythonic, and goes against the most basic programming principle of not doing needless things.

Now it is obvious from the comments that devs using PyYAML for actual work are negatively impacted by this anti-feature. Like in my case, it renders this library unusable for their projects.

Whoever wants the keys sorted should be doing it at the .yaml source. That is the whole point of defining interfaces in YAML.

"Backwards compatibility" feels like a weak excuse here.

Please listen to the community and be honest here.

@perlpunk
Copy link
Copy Markdown
Member Author

@gatopeich this is a pull request. It's a suggestion.

I don't decide anything, I can just help making decisions.

I agree that for python versions that keep the original order, sort_keys=False would be the better default.
For older versions I think that default wouldn't make sense, because getting an arbitrary sort order when dumping is less helpful than sorted keys. Plus it's not backwards compatible.

Would it make sense to make the default depend on the python version?
Thoughts?

@gatopeich
Copy link
Copy Markdown

Would it make sense to make the default depend on the python version?

Okay. Then let's apply same default as for json.dumps(): sort_keys=True until Python 2.6, False since v2.7.

@ofek
Copy link
Copy Markdown

ofek commented Dec 25, 2018

Can this be merged now?

@rr-
Copy link
Copy Markdown

rr- commented Jan 27, 2019

Would it make sense to make the default depend on the python version?
Thoughts?

If you're concerned about portability, it's okay to have it default to True. The important bit here is to be able to customize it at all, which we're still unable to.

(I think having a package changing its behavior after seemingly transparent Python upgrade breaks the principle of least astonishment, although personally I too would like to see it default to False.)

@perlpunk
Copy link
Copy Markdown
Member Author

Closing, recreated based on current master: #254

@perlpunk perlpunk deleted the dumper-sortkeys branch March 18, 2019 19:38
enzbang added a commit to enzbang/e3-aws that referenced this pull request May 9, 2019
enzbang added a commit to enzbang/e3-aws that referenced this pull request May 9, 2019
enzbang added a commit to enzbang/e3-aws that referenced this pull request May 9, 2019
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.

9 participants