python2.7: gensim 2.2.0: requirements state six>=1.5.0, but actually requires six>= 1.9 #1495
Comments
|
Sorry about that! Six 1.5.0 should be fine, it's just this module should be importing @menshikh-iv aren't we pinning the oldest supported versions of dependencies, for CI testing? Also, I don't think I wrote this code. Why does it say |
|
@piskvorky for CI we pinning only scipy/numpy versions for testing purposes. |
|
OK. Let's pin everything, to prevent issues like this. Ideally, we'd have a CI matrix of version combinations to test. We definitely want to test the earliest versions we support, and the latest (most recent). Something in between would be nice too, but not as critical. @HodorTheCoder @menshikh-iv can you fix this issue by removing the |
|
I'll try it. Thanks for the quick response. In the interim I fixed it by
downgrading to gensim 2.1.0, but it sounds like this is an easy fix.
…On Fri, Jul 21, 2017 at 8:56 PM, Radim Řehůřek ***@***.***> wrote:
OK. Let's pin everything, to prevent issues like this.
Ideally, we'd have a CI matrix of version combinations to test. We
definitely want to test the *earliest* versions we support, and the
*latest* (most recent). Something in between would be nice too, but not
as critical.
@HodorTheCoder <https://github.com/hodorthecoder> @menshikh-iv
<https://github.com/menshikh-iv> can you fix this issue by removing the
viewitems? iteritems enough, no need to introduce more restrictive
dependencies.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1495 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AOnd78Xmj5nOF-JMskq9yNqZKMC5eIRiks5sQVbOgaJpZM4OfnMr>
.
|
|
@piskvorky about old/newest versions - it's a good idea, but this will increase the test execution time significantly (from ~25 to 50 minutes for linux CI, from 1h to 2h for windows CI and so on). For this reason, we should to pinned only earliest versions I think. |
|
Computer time is cheap (~free). Also, why not run them in parallel? Is there any dependency? |
|
Changing from viewitems to iteritems fixes this, and tests pass. I didn't submit a pull request, but I can if you want. Thanks. |
|
Yes, PR please. Thanks a lot! |
|
Stupid question. I created a new branch, but can't seem to push it up to the mothership to to open a PR against-- insufficient privileges (permission denied). Do I need to fork instead, or just use another existing branch? (I'm using the https url, not ssh, as origin). Thanks. |
|
|
Perfect. Thanks.
…On Jul 26, 2017 1:25 AM, "Menshikh Ivan" ***@***.***> wrote:
@HodorTheCoder <https://github.com/hodorthecoder>
1. Fork gensim repo to your github account
2. Clone your fork to PC
3. Create a new branch git checkout -b my-new-feature
4. Make changes, commit it and push
5. Go to original repo and create PR
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1495 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AOnd7_u-Ib_6zUXY4Maf6pXyRtugEKmoks5sRtvRgaJpZM4OfnMr>
.
|
|
PR submitted. |
|
Resolved in #1508. |
Hello,
Gensim latest version from pip2 for python2.7 lists requirements for six to be >=1.5.0, but the import breaks with six=1.7.3, at:
from six import viewitems, string_typesin
/usr/local/lib/python2.7/dist-packages/gensim/topic_coherence/text/analysis.pyThe library "six" has no viewitems module until version 1.9-ish.
When you do --upgrade with pip it upgrades six to 1.10 which fixes it, but that doesn't solve the problem of the initial requirement being wrong.
Thanks.
Description
TODO: change commented example
Steps/Code/Corpus to Reproduce
Expected Results
Actual Results
Versions
The text was updated successfully, but these errors were encountered: