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

Should _comparator return NotImplemented instead of raising TypeError? #316

Closed
tomschr opened this issue Nov 5, 2020 · 1 comment
Closed
Assignees
Labels
Bug Error, flaw or fault to produce incorrect or unexpected results Question Unclear or open issue subject for debate Release_3.x.y Only for the major release 3
Projects

Comments

@tomschr
Copy link
Member

tomschr commented Nov 5, 2020

Situation

when comparing an incompatible type, the Version class raises TypeError (through the _comparator decorator):

>>> class Foo:
...    pass

>>> v1 = semver.version.Version(1, 2, 3)
>>> v1 < Foo()
Traceback (most recent call last)
...
TypeError: other type <class '__main__.Foo'> must be in (<class 'semver.version.Version'>, <class 'dict'>, <class 'tuple'>, <class 'list'>, <class 'str'>, <class 'bytes'>)

From a semantic point of view this is correct.

Question

According to the NotImplemented documentation (emphasize by me):

Special value which should be returned by the binary special methods (e.g. __eq__(), __lt__(), __add__(), __rsub__(), etc.) to indicate that the operation is not implemented with respect to the other type; may be returned by the in-place binary special methods (e.g. __imul__(), __iand__(), etc.) for the same purpose. It should not be evaluated in a boolean context.

Should we return NotImplemented instead of raising TypeError?

@python-semver/reviewers @tlaferriere Thoughts? Concerns?

@tomschr tomschr added Release_3.x.y Only for the major release 3 Bug Error, flaw or fault to produce incorrect or unexpected results Question Unclear or open issue subject for debate labels Nov 5, 2020
@tomschr tomschr changed the title Should _comparator raise NotImplemented instead of TypeError? Should _comparator return NotImplemented instead of raising TypeError? Nov 5, 2020
@tomschr tomschr added this to To do in Release 3 via automation Nov 5, 2020
@tomschr tomschr self-assigned this Nov 5, 2020
@tomschr tomschr mentioned this issue Nov 5, 2020
4 tasks
@tlaferriere
Copy link
Contributor

I think this would make sense since this function is meant to be used in the __gt__ magic method and family. I also don't see much of a problem changing this because we're making it private.

Sounds good to me overall.

@tomschr tomschr closed this as completed in 6793320 Nov 8, 2020
tomschr added a commit that referenced this issue Nov 8, 2020
Fix #316: Return NotImplemented for comparisons
Release 3 automation moved this from To do to Done Nov 8, 2020
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 Question Unclear or open issue subject for debate Release_3.x.y Only for the major release 3
Projects
No open projects
Release 3
  
Done
Development

No branches or pull requests

2 participants