Skip to content

add OrderedDict for preserving key order#2

Closed
beygi wants to merge 3 commits intouiri:masterfrom
beygi:master
Closed

add OrderedDict for preserving key order#2
beygi wants to merge 3 commits intouiri:masterfrom
beygi:master

Conversation

@beygi
Copy link
Copy Markdown

@beygi beygi commented Mar 2, 2013

No description provided.

@uiri
Copy link
Copy Markdown
Owner

uiri commented Mar 2, 2013

I don't want to break compatibility with python 2.6 (and potentially earlier) for this. Do you have a compelling argument in favour of ordering dictionaries?

@untitaker
Copy link
Copy Markdown

@uiri You still can use the ordereddict package from PyPI

@beygi
Copy link
Copy Markdown
Author

beygi commented Mar 2, 2013

preserving orders is an extra feature and it's useful when we want to doing jobs in orders that exist in toml file so its nice to preserving orders by default

@uiri
Copy link
Copy Markdown
Owner

uiri commented Mar 2, 2013

preserving orders is an extra feature and it's useful when we want to doing jobs in orders that exist in toml file so its nice to preserving orders by default
I'm not sure I understand the use case. Do you mean iterating over the dictionary as if it were an array? Though, If you can make sure that python 2.6 users are taken care of, I see no reason to reject the patch.

I'm not sure how to handle the dependency of the ordereddict package which @untitaker mentioned.

@untitaker
Copy link
Copy Markdown

You just can check for the version inside the setup.py and supply the setup function with the dependency if neccessary. This is how i have handled it in untitaker/python-webuntis and it works quite well.

@untitaker
Copy link
Copy Markdown

Unfortunately the official spec from mojombo/toml doesn't indicate whether hashes have order or not.

@beygi
Copy link
Copy Markdown
Author

beygi commented Mar 2, 2013

@untitaker i'm not familiar with setuptools , can you fix it ?

@beygi
Copy link
Copy Markdown
Author

beygi commented Mar 2, 2013

@untitaker i inform mojombo about this issue

@untitaker
Copy link
Copy Markdown

@beygi Is it okay if i make a pull request to your fork, so it shows up in this one?

@beygi
Copy link
Copy Markdown
Author

beygi commented Mar 2, 2013

@untitaker its okay , when your work is done i send a new pull request . thank you :)

@untitaker
Copy link
Copy Markdown

If you merge beygi#1, the changes will show up here.

Use ordereddict from PyPI for Python < 2.7
@untitaker
Copy link
Copy Markdown

I just tested this because i wasn't sure if distutils behaved as documented -- and surprise, it ignores the requires argument completely. If you allow, i will create another pull request on @beygi's fork, which switches from distutils to setuptools. Is this okay for you @uiri?

@untitaker
Copy link
Copy Markdown

And it seems that this point has been made unneccessary by the response in toml-lang/toml#162 anyway.

@uiri
Copy link
Copy Markdown
Owner

uiri commented Mar 2, 2013

Given that mojombo has stated not to preserve order (or at least that it is not a requirement), I'm closing this pull request.

@uiri uiri closed this Mar 2, 2013
@beygi
Copy link
Copy Markdown
Author

beygi commented Mar 2, 2013

@uiri this is an extra feature ! and its good to keep it :)

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