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

Added DuplicateOrderedDict #6

Closed
wants to merge 11 commits into from
Closed

Added DuplicateOrderedDict #6

wants to merge 11 commits into from

Conversation

SleepProgger
Copy link
Contributor

I added your DuplicateOrderedDict implementation as referenced in #5 , and some test cases.
Also, i made the usage feel a bit more like a "real" dict.

If you don't like my changes please tell me what to change.
IMHO, the vdf lib really need something like this build in asap.

@rossengeorgiev
Copy link
Contributor

If we are to ship this we need to implement all the methods from https://docs.python.org/3/library/stdtypes.html#mapping-types-dict

Should be compatible with py2 and py3. For example, in py3 .values() returns a list, while in py3 you get a iterator. To get a iterator in py2 you need .itervalues(). It would be nice adhere to that API.

All that with the order/duplicates in mind, of course. At a glance it seems that it might not be the case currently.

May as well consider calling it VDFDict or something else as it's specific to VDF, rather than a generic solution. For example, one of the underline assumptions is that keys can only be a str or tuple.

Finally, might as well throw the code for it in a separate file as this is __init__ is getting long.

@SleepProgger
Copy link
Contributor Author

I refactored the code a bit. It should now be compatible with py2 and 3.
Also moved the class to its own file.
Let me know what you think.

edit: We should def squash my fail series if you accept this pull request. (Sorry for the mess)

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.7%) to 99.346% when pulling 448a357 on SleepProgger:duplicate_dict into 8e50d5a on ValvePython:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-4.2%) to 95.806% when pulling 14d84fe on SleepProgger:duplicate_dict into 8e50d5a on ValvePython:master.

@rossengeorgiev
Copy link
Contributor

Hi, I've pulled your code and reworked it. See #8

This pull request was closed.
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