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

Do test instructions still work? #2788

Closed
piskvorky opened this issue Apr 9, 2020 · 9 comments · Fixed by #2814
Closed

Do test instructions still work? #2788

piskvorky opened this issue Apr 9, 2020 · 9 comments · Fixed by #2814
Labels
bug Issue described a bug documentation Current issue related to documentation impact LOW Low impact on affected users reach LOW Affects only niche use-case users testing Issue related with testing (code, documentation, etc)
Milestone

Comments

@piskvorky
Copy link
Owner

In our README, we tell users to run python setup.py test.

One user reported on our mailing list that this doesn't actually work.

We should fix the docs or the tests – whichever is at fault here.

@piskvorky piskvorky added bug Issue described a bug documentation Current issue related to documentation testing Issue related with testing (code, documentation, etc) impact LOW Low impact on affected users reach LOW Affects only niche use-case users labels Apr 9, 2020
@piskvorky piskvorky added this to the 3.8.3 milestone Apr 24, 2020
@piskvorky
Copy link
Owner Author

piskvorky commented Apr 24, 2020

@mpenkov apparently Gensim in develop requires py3.5+ already:
https://github.com/RaRe-Technologies/gensim/blob/develop/setup.py#L23

That means we cannot work off develop for 3.8.3, because that must still support py2.7. Which branch should I use?

@piskvorky
Copy link
Owner Author

Worse: does 3.8.2 have the same problem too? I don't see any py2.7 wheels on PyPI:
https://github.com/RaRe-Technologies/gensim/blob/develop/setup.py#L23

@piskvorky
Copy link
Owner Author

piskvorky commented Apr 24, 2020

I tracked the problem down to commit e859c11 (part of PR #2630, merged 6 months ago). It claims to remove native Python implementations of Cython extensions, but also silently drops support for Python2 (!).

@mpenkov I'm not sure how to proceed, so I'll pause work on 3.8.3. Silently dropping Python2 support in a minor bugfix release is terrible, we cannot do that.

3.8.2 seems thoroughly broken, let's retract that release / supercede it ASAP.

@mpenkov
Copy link
Collaborator

mpenkov commented Apr 27, 2020

Created #2811 to deal with the Py2 issue.

@mpenkov
Copy link
Collaborator

mpenkov commented Apr 28, 2020

@mpenkov I'm not sure how to proceed, so I'll pause work on 3.8.3.

@piskvorky We've successfully patched our develop HEAD to work on Py2 (#2812), so please proceed as previously discussed.

@piskvorky
Copy link
Owner Author

Great, will do. Thanks @mpenkov @menshikh-iv

@piskvorky
Copy link
Owner Author

We've successfully patched our develop HEAD

@mpenkov I don't see that in develop. Should I work off the release-3.8.3 branch?

@mpenkov
Copy link
Collaborator

mpenkov commented Apr 28, 2020

Because the changes for Py2.7 are essentially only 3.8.3, we won't be merging them into develop (there is no point, because we'd be removing them as soon as we ditch Py2.7).

Please work off develop. We will merge that into 3.8.3 before the actual release (as opposed to the other way around).

@menshikh-iv
Copy link
Contributor

btw about original topic: python setup.py test no relevant for last years, should be tox -e py<version>-<os>, like tox -e py37-linux

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue described a bug documentation Current issue related to documentation impact LOW Low impact on affected users reach LOW Affects only niche use-case users testing Issue related with testing (code, documentation, etc)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants