podhmo / python-node-semver

python version of node-semver
MIT License
21 stars 15 forks source link

max_satisfying doesn't work with byte-strings on Python3 #26

Closed jeverling closed 5 years ago

jeverling commented 5 years ago

Using Python 3, if you pass byte-strings to max_satisfying, it returns None.

The reason is a TypeError: cannot use a string pattern on a bytes-like object exception in semver#L740, however this exception is masked by a bare except statement in semver#L1079, that leads to max_satisfying returning None.

I think it would be good to document that byte-strings need to be decoded before they can be passed to semver. Alternatively, the library could try to decode byte-strings before using them, which should be reasonably easy because semvers are supposed to be ascii only.

podhmo commented 5 years ago

oh, returning none is bad behavior. at least, should be error is raised

podhmo commented 5 years ago

Honestly, I don't like treating bytes type as string. But, so, semver format is not including multi bytes char, normaly.

jeverling commented 5 years ago

I think it's enough to raise the error, it is then up to the user to make sure to pass strings not bytes. But at the moment it's not easy to see why it doesn't work.

podhmo commented 5 years ago

0.6.0 is released

from semver import max_satisfying

print(max_satisfying([b"1.0.0"], "1.0.0"))
# semver.InvalidTypeIncluded: must be str, but b'1.0.0'

print(max_satisfying(["1.0.0"], b"1.0.0"))
# semver.InvalidTypeIncluded: must be str, but b'1.0.0'
jeverling commented 5 years ago

Thank you for the quick fix!