Skip to content

Py2 vs. Py3 incompatibility: TypeError #274

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

Closed
tomschr opened this issue Jul 6, 2020 · 13 comments · Fixed by #275
Closed

Py2 vs. Py3 incompatibility: TypeError #274

tomschr opened this issue Jul 6, 2020 · 13 comments · Fixed by #275
Labels
Bug Error, flaw or fault to produce incorrect or unexpected results

Comments

@tomschr
Copy link
Member

tomschr commented Jul 6, 2020

Situation

I'm confused by the statement in the Readme that seems to say 2.10.0 does still support Python 2.7. That's not what I'm seeing. I have a project that uses this package and runs in Py2, and as soon as it started using the 2.10 release, I started getting errors like this which look to me like what you'd get from a Py2 vs. Py3 incompatibility:

TypeError("Expected str or VersionInfo instance, but got <type 'unicode'>"

When I pin the dependency to <2.10, the error goes away.

Originally posted by @eli-darkly in #161 (comment)

@tomschr tomschr added the Bug Error, flaw or fault to produce incorrect or unexpected results label Jul 6, 2020
@tomschr
Copy link
Member Author

tomschr commented Jul 6, 2020

@eli-darkly Eli, do you have a code example where this error happens? Thanks! 👍

@eli-darkly
Copy link
Contributor

@tomschr, that's a very reasonable request but it's not feasible to post the actual project, so I'll try to put together a minimal test case. I wanted to ask first with my comment on the other issue, in case I just misunderstood and shouldn't have expected this to work.

@tomschr
Copy link
Member Author

tomschr commented Jul 6, 2020

Thanks Eli! Actually I found an example:

$ python2
>>> import semver
>>> semver.compare("1.1.0", u"1.2.2")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "semver.py", line 100, in wrapper
    return func(*args, **kwargs)
  File "semver.py", line 779, in compare
    return v1.compare(ver2)
  File "semver.py", line 427, in compare
    cls.__name__, type(other)
TypeError: Expected str or VersionInfo instance, but got <type 'unicode'>

This is a bug, of course, and shouldn't happen.

@tomschr
Copy link
Member Author

tomschr commented Jul 9, 2020

@eli-darkly Do you have an idea how to test this?

@eli-darkly
Copy link
Contributor

@tomschr Well, at first I was going to try to put together a minimal test case, but I stopped working on that when you posted your last comment since it seemed like you had already found one. So I'm a little confused. Is your example not sufficient?

@tomschr
Copy link
Member Author

tomschr commented Jul 10, 2020

@eli-darkly Sorry if I wasn't clear. The example is fine, but I'm stuck on the practical side: How do I create a real test case that can be run from Python 2 and Python 3? Or is one test for both versions impossible and I should create two separate test cases?

The problem that I face are the string types: Python 2 has str and unicode whereas Python 3 has str and bytes. So basically, it boils down to this:

def test_should_work_with_string_and_unicode():
    result = semver.compare("1.1.0", "1.2.2")  # <= (1)
    assert result == -1

I need to make sure, that argument 1 ("1.1.0") is another type as argument 2 ("1.2.2"). In Python3 it would be easy, I would just write: semver.compare("1.1.0", b"1.2.2"); likewise in Python2 I would write: semver.compare(u"1.1.0", "1.2.2"). But these are abbreviations and b is not available in Python2.

How to change the line (1) so it works for both Python versions?

@eli-darkly
Copy link
Contributor

Oh yeah, I see. I guess my first impulse would be not to try to make the same test code run in Py2 and Py3. Like, put those in two different files and choose to load one of them based on the runtime version.

@tomschr
Copy link
Member Author

tomschr commented Jul 10, 2020

I thought of that too. However, the six project has solved that problem too, so I think it should be possible.

Another idea was to try create marks which can be skip the wrong Python version. Something like this:

import sys
import pytest

PY2 = sys.version_info[0] == 2
PY3 = sys.version_info[0] == 3

requires_py2 = pytest.mark.skipif(PY2,
                                  reason="test requires python2")
requires_py3 = pytest.mark.skipif(PY3,
                                  reason="test requires python3")

and then you can mark your test accordingly:

@require_py3
def test_should_work_with_string_and_unicode():
    # ...

However, that still leaves the open question how to mark the arguments.

@eli-darkly
Copy link
Contributor

eli-darkly commented Jul 10, 2020

I haven't tried to do that kind of thing with pytest, but my guess would be that it won't work. The Py2 interpreter will still try to parse that code, even if the test doesn't get executed, so any Py3-only syntax will cause an error. That's why I was thinking of putting the code in two different files, and conditionally importing one.

tomschr added a commit to tomschr/python-semver that referenced this issue Jul 10, 2020
This fixes problems between different string types. In Python2
str vs. unicode and in Python3 str vs. bytes.
tomschr added a commit to tomschr/python-semver that referenced this issue Jul 10, 2020
This fixes problems between different string types. In Python2
str vs. unicode and in Python3 str vs. bytes.

* Add some code from six project
* Suppress two flake8 issues (false positives)
* docformatter wanted different linebreaks at the
  beginning of a docstring
tomschr added a commit to tomschr/python-semver that referenced this issue Jul 10, 2020
This fixes problems between different string types. In Python2
str vs. unicode and in Python3 str vs. bytes.

* Add some code from six project
* Suppress two flake8 issues (false positives)
tomschr added a commit to tomschr/python-semver that referenced this issue Jul 10, 2020
This fixes problems between different string types. In Python2
str vs. unicode and in Python3 str vs. bytes.

* Add some code from six project
* Suppress two flake8 issues (false positives)
@tomschr
Copy link
Member Author

tomschr commented Jul 10, 2020

I haven't tried to do that kind of thing with pytest, but my guess would be that it won't work.

Seems it does. 😉 I took inspirations from the six project. Hope I didn't forget something important. Maybe the name of the test file (test_py2.py) could be improved, but I was not very creative this morning. 😉

I was thinking of putting the code in two different files [...]

That would be my last resort. 😉 First I would like to try it with one file.

tomschr added a commit to tomschr/python-semver that referenced this issue Jul 22, 2020
This fixes problems between different string types. In Python2
str vs. unicode and in Python3 str vs. bytes.

* Add some code from six project
* Suppress two flake8 issues (false positives)
@tomschr
Copy link
Member Author

tomschr commented Jul 22, 2020

@gsakkis Could you have a look, George? That would be wonderful! Thanks! 👍

@gsakkis
Copy link
Contributor

gsakkis commented Jul 23, 2020

@tomschr to be honest I'd either drop python 2 support (at least for the next major semver release) or use six as a dependency instead of copy-pasting parts of it.

@tomschr
Copy link
Member Author

tomschr commented Jul 23, 2020

Thanks @gsakkis for your ideas. 👍

to be honest I'd either drop python 2 support

That was also my thought at first. However, I think it would make sense to have this fix in semver2 too.

or use six as a dependency instead of copy-pasting parts of it.

I was thinking about six, but I would like to avoid introducing another dependency. Especially if this development line (semver2) will be deprecated soon and we will switch to semver3.

The parts that are coming from six are really very small. All in all it's ~23 lines of code. It's probably easy to remove them once we release semver3.

tomschr added a commit to tomschr/python-semver that referenced this issue Oct 16, 2020
This fixes problems between different string types. In Python2
str vs. unicode and in Python3 str vs. bytes.

* Add some code from six project
* Suppress two flake8 issues (false positives)
tomschr added a commit to tomschr/python-semver that referenced this issue Oct 17, 2020
This fixes problems between different string types. In Python2
str vs. unicode and in Python3 str vs. bytes.

* Add some code from six project
* Suppress two flake8 issues (false positives)
* Update Changelog
* Update CONTRIBUTORS
* Document creating a version from a byte string

Co-authored-by: Eli Bishop <eli-darkly@users.noreply.github.com>
tomschr added a commit to tomschr/python-semver that referenced this issue Oct 17, 2020
This fixes problems between different string types. In Python2
str vs. unicode and in Python3 str vs. bytes.

* Add some code from six project
* Suppress two flake8 issues (false positives)
* Update Changelog
* Update CONTRIBUTORS
* Document creating a version from a byte string

Co-authored-by: Eli Bishop <eli-darkly@users.noreply.github.com>
tomschr added a commit that referenced this issue Oct 17, 2020
This fixes problems between different string types. In Python2
str vs. unicode and in Python3 str vs. bytes.

* Add some code from six project
* Suppress two flake8 issues (false positives)
* Update Changelog
* Update CONTRIBUTORS
* Document creating a version from a byte string

Co-authored-by: Eli Bishop <eli-darkly@users.noreply.github.com>
@tomschr tomschr mentioned this issue Oct 17, 2020
3 tasks
tomschr added a commit to tomschr/python-semver that referenced this issue Oct 17, 2020
* python-semver#274 / python-semver#275 String Types Py2 vs. Py3 compatibility
* python-semver#277 Turn VersionInfo.parse into classmethod to allow subclasses
* python-semver#286 Add author and update changelog for python-semver#276/python-semver#277
tomschr added a commit that referenced this issue Oct 17, 2020
* #274 / #275 String Types Py2 vs. Py3 compatibility
* #277 Turn VersionInfo.parse into classmethod to allow subclasses
* #286 Add author and update changelog for #276/#277
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Error, flaw or fault to produce incorrect or unexpected results
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants