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

Remove viewitems() as it breaks compatibility with python<2.7 #81

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

foobacca
Copy link
Contributor

It was introduced in commit 808768c -
adding support for Solr's JSON API.

viewitems() was backported from Python 3 items() into python 2.7, but if
we use it then python 2.6 and older will fail.

It was introduced in commit 808768c -
adding support for Solr's JSON API.

viewitems() was backported from Python 3 items() into python 2.7, but if
we use it then python 2.6 and older will fail.
@evansd
Copy link
Contributor

evansd commented Feb 11, 2014

Hi @foobacca, these are only used in the JSON methods so the original XML API should still be Python 2.6 compatible.

We (I work with @tow, the original author) added support for the JSON API because we're working with very large results sets and need to be as efficient (in terms of CPU and memory usage) as possible. For this reason we'd like to stick with using viewitems() to avoid the overhead of creating new lists.

The JSON API doesn't offer any additional features over the XML one, just better performance with large result sets, so if you're stuck on Python 2.6 I'd suggest just sticking with the XML API.

@foobacca
Copy link
Contributor Author

Would iteritems() (python 2.2+) be an acceptable compromise? That is still a generator. The main difference between viewitems() and iteritems() that I can see, is that when you use viewitems() the generator will reflect changes to the underlying dict after the for loop has started. But I don't see any advantage to that in the places you're using them.

If you're happy with iteritems() then let me know and I'll do another pull request.

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