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

Use six.iteritems to support Python 2&3 #5

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

blag
Copy link
Contributor

@blag blag commented Aug 8, 2016

No description provided.

@blag
Copy link
Contributor Author

blag commented Aug 8, 2016

If you have a PyPI account, let me know your username and I'll add you as an/the owner of the markdown-icons and push a release to it so people can pip install markdown-icons! 😄

@MadLittleMods
Copy link
Owner

Thanks @blag

Have you run the tests with Python 2 and 3? (probably should get some CI on this at some point)

What version of python, does markdown-icons currently not run on?

@blag
Copy link
Contributor Author

blag commented Aug 11, 2016

It currently runs in both, this just makes it support Python 2 a bit better, because dict.items() in Python 2 returns a list (could be big), and dict.items() in Python 3 returns an iterator. To get an iterator in Python 2, you could call dict.iteritems(), but that method doesn't exist in Python 3.

So, in order to return an iterator in both Python 2 and 3, six has an iteritems() function, which is what we use here.

It's a two line change (including the import six line), and as long as it's syntactically correct (it is), it will run in both Python 2 and 3. But, since it's only a performance enhancement to code that probably won't ever be a bottleneck, this patch very well might be premature optimization. I just like to do things The Right Way.

Cheers! 😄

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.

2 participants