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.

Sign up for GitHub

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

Jump to bottom

PyHamcrest 1.10.0 is not py2-compatible. It should set python_requires>=3 to avoid breaking users who are still on Py2. #131

Closed
tvalentyn opened this issue Jan 7, 2020 · 16 comments · Fixed by #132
Closed

PyHamcrest 1.10.0 is not py2-compatible. It should set python_requires>=3 to avoid breaking users who are still on Py2. #131

tvalentyn opened this issue Jan 7, 2020 · 16 comments · Fixed by #132

Comments

@tvalentyn
Copy link

tvalentyn commented Jan 7, 2020 •
edited

import hamcrest no longer works on Python 2 with pyhamcrest==1.10.0.

To avoid installation of pyhamcrest==1.10.0 and newer versions on Python 2, it would be better to add python_requires>=3.0 stanza in setup.py starting from 1.10.0. To fix this for 1.10.0, we would need to update already released 1.10.0 pypi artifacts.

As of now, all python 2 users who install pyhamcrest without restricting the version to 1.9.0, will be broken.

A better user experience would be if Python 2 users get the last version of pyhamcrest that is supported on Python 2. This is convention is often followed by major python libraries, for example numpy.
@tvalentyn tvalentyn changed the title PyHamcrest 1.10.0 is not py2-compatible. It should set python_requires>3 PyHamcrest 1.10.0 is not py2-compatible. It should set python_requires>=3 to avoid breaking users who are still on Py2. Jan 7, 2020
@brunns
Copy link
Member

brunns commented Jan 7, 2020 via email

@twm twm mentioned this issue Jan 8, 2020
Merged
5 tasks
twm added a commit to twisted/twisted that referenced this issue Jan 8, 2020
@twm
Blacklist PyHamcrest 1.10.0
9638494 
Work around hamcrest/PyHamcrest#131
@twm twm mentioned this issue Jan 8, 2020
Merged
@twm
Copy link
Contributor

twm commented Jan 8, 2020

For the benefit of those searching, this causes Python 2.7 tests to fail with a SyntaxError like this:

Traceback (most recent call last):
  File "/home/travis/build/twisted/twisted/build/py27-alldeps-withcov-posix/lib/python2.7/site-packages/twisted/trial/runner.py", line 531, in loadPackage
    module = modinfo.load()
  File "/home/travis/build/twisted/twisted/build/py27-alldeps-withcov-posix/lib/python2.7/site-packages/twisted/python/modules.py", line 392, in load
    return self.pathEntry.pythonPath.moduleLoader(self.name)
  File "/home/travis/build/twisted/twisted/build/py27-alldeps-withcov-posix/lib/python2.7/site-packages/twisted/python/reflect.py", line 308, in namedAny
    topLevelPackage = _importAndCheckStack(trialname)
  File "/home/travis/build/twisted/twisted/build/py27-alldeps-withcov-posix/lib/python2.7/site-packages/twisted/python/reflect.py", line 247, in _importAndCheckStack
    return __import__(importName)
  File "/home/travis/build/twisted/twisted/build/py27-alldeps-withcov-posix/lib/python2.7/site-packages/twisted/test/test_unix.py", line 22, in <module>
    from twisted.test.test_tcp import MyServerFactory, MyClientFactory
  File "/home/travis/build/twisted/twisted/build/py27-alldeps-withcov-posix/lib/python2.7/site-packages/twisted/test/test_tcp.py", line 13, in <module>
    import hamcrest
  File "/home/travis/build/twisted/twisted/build/py27-alldeps-withcov-posix/lib/python2.7/site-packages/hamcrest/__init__.py", line 2, in <module>
    from hamcrest.library import *
  File "/home/travis/build/twisted/twisted/build/py27-alldeps-withcov-posix/lib/python2.7/site-packages/hamcrest/library/__init__.py", line 7, in <module>
    from hamcrest.library.object import *
  File "/home/travis/build/twisted/twisted/build/py27-alldeps-withcov-posix/lib/python2.7/site-packages/hamcrest/library/object/__init__.py", line 4, in <module>
    from .hasproperty import has_properties, has_property
exceptions.SyntaxError: invalid syntax (hasproperty.py, line 174)

You can work around this by blacklisting this version in your setup.py or requirements file, like this:

PyHamcrest >= 1.9.0, != 1.10.0

This is under the assumption that the next release will add the appropriate metadata. See #132.

@offbyone
Copy link
Member

offbyone commented Jan 8, 2020 •
edited

@brunns we need to yank 1.10 for now til this can be fixed. 1.10 is not supposed to drop Python 2 support... unless you have a fix in the wings.

@brunns
Copy link
Member

brunns commented Jan 8, 2020 •
edited

I don't have the rights to yank PyHamcrest releases on PyPI - can you promote me? I need to be a "Maintainer".

@brunns
Copy link
Member

brunns commented Jan 8, 2020 •
edited

There is a problem with keeping 1.10 Python 2 compatible. The main purpose of 1.10 in my mind is to let folks see warnings from deprecated methods, making it easier to move to a version 2.0 where they will be removed. But some of the deprications were marked after Python 2 support was dropped.

Perhaps we just shouldn't bother with a 1.10 at all?

@offbyone
Copy link
Member

offbyone commented Jan 8, 2020 via email

@brunns
Copy link
Member

brunns commented Jan 8, 2020 via email

@brunns brunns reopened this Jan 8, 2020
@craigds craigds mentioned this issue Jan 8, 2020
Merged
@ahvigil
Copy link

ahvigil commented Jan 9, 2020

this breaks builds for those using pipenv who have lockfiles produced when pyhamcrest 1.10.0 was available:

ERROR: No matching distribution found for pyhamcrest==1.10.0

in the future please don't unpublish releases as it breaks build determinism for anyone using a build system like pipenv or poetry

@offbyone
Copy link
Member

offbyone commented Jan 9, 2020

I'm gonna give you the benefit of the doubt that your tone is only accidentally seeming like it's telling us what to do. In the future please consider how your choice of words reflects your desires so as not to cause this confusion again.

I knew what removing the version would do, and made a conscious choice... just as I would in any other event like this.

@brunns
Copy link
Member

brunns commented Jan 10, 2020 via email

@ahvigil
Copy link

ahvigil commented Jan 10, 2020

I'm gonna give you the benefit of the doubt that your tone is only accidentally seeming like it's telling us what to do. In the future please consider how your choice of words reflects your desires so as not to cause this confusion again.

I knew what removing the version would do, and made a conscious choice... just as I would in any other event like this.

no reason to be defensive or rude- I made a completely reasonable request which you are of course free to ignore

@brunns
Copy link
Member

brunns commented Jan 10, 2020

So, how should we move forward on this? I see three options:

A - Cut a Python 2 compatible 1.10.1 release, cut from immediately the removal of Python 2 support, with any deprications back-ported.
B - Cut a non Python 2 compatible 1.10.1 release, looking like 1.10.0, but with metadata corrected., and one defect fix backported from trunk.
C - Go straight to a non Python 2 compatible 2.0 release.

I'm sure there are options I've missed, so please let me know what they are.

Of the 3, I lean towards option B if we can live with a non Python 2 compatible release in the 1.x series. I know there are objections to that, but I'm not sure I understand what they are.

Option C would be my 2nd choice, though it would make upgrading to 2.0 more difficult, since some methods would disappear without warnings.

Option A is a fair amount of work, and I'm not sure I see the need since presumably Python 2 users wouldn't really be looking for new features or changes now that Python 2 is EOL.

Thoughts?

@offbyone
Copy link
Member

If we can, I would prefer A; I think we need a version of 1.x that supports Python 2 in the 1.10 stream to capstone that minor version.

2.0.0 can then immediately be rolled out as well.

@brunns
Copy link
Member

brunns commented Jan 15, 2020

I've release a Python 2/3 compatible version 1.10.1. Fingers crossed. Let me know if there are any issues with it.

@brunns
Copy link
Member

brunns commented Jan 16, 2020 •
edited

Seems quieter this time. OK to close?

@offbyone
Copy link
Member

offbyone commented Jan 16, 2020 via email

@brunns brunns closed this as completed Jan 16, 2020
@tvalentyn tvalentyn mentioned this issue Feb 26, 2020
Merged
rodrigc pushed a commit to twisted/twisted that referenced this issue Mar 28, 2020
@twm @rodrigc
Blacklist PyHamcrest 1.10.0
7615e93 
Work around hamcrest/PyHamcrest#131
@tvalentyn tvalentyn mentioned this issue Jun 11, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Python version compatibility metadata twm/PyHamcrest
5 participants
@twm @offbyone @brunns @ahvigil @tvalentyn