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

Improve numbers #464

Closed
wants to merge 8 commits into from
Closed

Improve numbers #464

wants to merge 8 commits into from

Conversation

tharvik
Copy link
Contributor

@tharvik tharvik commented Aug 10, 2016

https://docs.python.org/3/library/numbers.html

  • merge both, add types
  • remove from pytype tests, because it doesn't support # type: ignore comments
  • add SupportsComplex to typing.py in py2

@gvanrossum
Copy link
Member

@matthiaskramm If you are okay with adding numbers to the pytype blacklist, I'm okay with this change and you can merge it.

@tharvik Thanks for adding SupportsComplex to the typing.pyi stubs!

@tharvik
Copy link
Contributor Author

tharvik commented Aug 10, 2016

@matthiaskramm to be precise, the issue is occuring for def __complex__(self) -> Complex: ... # type: ignore because the definition expected from SupportsComplex is def __complex__(self) -> complex.

@matthiaskramm
Copy link
Contributor

Can we make complex inherit from numbers.Complex and change SupportsComplex to do def __complex__(self) -> Complex?

@gvanrossum
Copy link
Member

gvanrossum commented Aug 15, 2016 via email

@matthiaskramm
Copy link
Contributor

Played around with it, but for this to work, you'd have to make typing import numbers. Of course, numbers imports typing, so you have a circular dependency which, it seems, mypy isn't too happy with.

All in all, it seems less painful to just use the # type: ignore.

@JukkaL
Copy link
Contributor

JukkaL commented Aug 17, 2016

A small experiment indicates that complex(x) expects x.__complex__() to return a concrete complex object, so having complex, not Complex as the return type of __complex__ seems better to me.

@gvanrossum
Copy link
Member

Hey @tharvik -- I haven't followed this PR at all -- can you summarize why it is failing with mypy?

@tharvik
Copy link
Contributor Author

tharvik commented Aug 29, 2016

@gvanrossum I'm trying to have builtins int and such depending on numbers.Integral (and so on) but I'm failing, I don't think that I'm taking the best way to do it.

The issue that I'm having is that, correct me if I'm wrong, each class from numbers do not respect Liskov. For example, Integral derive from Real, but Integral.__add__ do not accept Real but Real.__add__ does. And int.__add__, only accept int. So I don't really see a nice way to write it, without type: ignore or other tricks.

@gvanrossum
Copy link
Member

Hm, that's strange. Integral.add should accept a Real, the result will
just not be Integral.

And int.add does accept float (at least conceptually), again returning
float (it's an overload). The existing annotations are not completely
correct (and mypy has some additional int->float magic that makes it go
away).

However I don't think anyone uses these ABCs, so perhaps it's not so
important?

@JukkaL
Copy link
Contributor

JukkaL commented Aug 29, 2016

@gvanrossum We discussed this back in 2013 before PEP 484 and I believe the conclusion was that the classes in numbers are not going to be very useful for type checking and we shouldn't use them in stdlib stubs. I'm still against using numbers in builtins, and for the same reasons as before. I'm happy to elaborate or look up the old discussions -- I've written some points below.

For example, int.__add__ accepts concrete float objects but not arbitrary Real instances (or even Integral). Annotating it as accepting Integral seems like a lie (from a type system perspective). I don't quite see how int could safely be a subtype of Integral, unless we somehow extend the type system to be able to express more (not sure what, exactly) about the semantics of operator methods. Basically the PEP 484 type system is probably not powerful enough, and I'm not convinced that it's worthwhile to try to extend it to support this.

Basically none of the functions in stdlib stubs accept Integral values as arguments, so what you can do with them in statically typed code is pretty limited. A lot of functions only work with concrete int / float objects anyway, so replacing int with Integral in the stubs is generally not good.

Another reason I don't like numbers is that if we get it to work somehow, some users are going to try to annotate everything with Integral / Real (as they are more abstract / duck typing) and then they are surprised when they hit issues -- as they will likely hit, since built-in functions generally don't work with them. If some subsets of the user population will write code using Integral and others using int, these codebases generally would not be able to interact seamlessly because Integral is not a subtype of int.

@tharvik Do you have a concrete use case that this solves? I'd like to see some non-trivial example code that is improved by using Integral / Real as defined in the PR.

@JukkaL
Copy link
Contributor

JukkaL commented Aug 29, 2016

(This topic comes up pretty frequently and is quite non-trivial -- maybe we should discuss this in the typing issue tracker, make a formal decision and write it up in PEP 484.)

@gvanrossum
Copy link
Member

gvanrossum commented Aug 29, 2016 via email

@JukkaL
Copy link
Contributor

JukkaL commented Aug 29, 2016

Further discussion here: python/typing#272

@tharvik
Copy link
Contributor Author

tharvik commented Aug 30, 2016

@JukkaL I don't find the codebase where I saw it,.

For now, let's close it, I'll let you know if I come across some code really using it.

@tharvik tharvik closed this Aug 30, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants