Why does mypy consider...
class Foo:
def factory(self) -> str:
return 'Hello'
class Bar(Foo):
def factory(self) -> int:
return 10
to be an error? While it may be poor design, it isn't wrong.
Thanks,
Phil
It violates the Liskov Substitution Principle. You may want to Google that. In particular the problem is that if we have a Foo instance and we call its factory() method, the typechecker expects that it returns a str; but when that Foo instance is actually a Bar instance, this assumption is incorrect. Note that isinstance(x, Foo) is still true if x is a Bar instance.
On 24 Feb 2016, at 7:32 pm, Guido van Rossum [email protected] wrote:
It violates the Liskov Substitution Principle. You may want to Google that. In particular the problem is that if we have a Foo instance and we call its factory() method, the typechecker expects that it returns a str; but when that Foo instance is actually a Bar instance, this assumption is incorrect. Note that isinstance(x, Foo) is still true if x is a Bar instance.
What does that mean regarding the effectivness of mypy on code that exhibits that sort of behaviour? Is it still useful?
My actual use case is in stub files describing a C++ API wrapped in Python. factory() is non-virtual in both Foo and Bar - they are completely unrelated and just happen to have the same name.
Is there a case for treating stub files differently, maybe by providing something like @overload that tells mypy that Bar.factory() is not an override of Foo.factory()?
Phil
You can also use # type: ignore in the subclass method definition and mypy will stop complaining about it.
Python doesn't have the concept of non-virtual functions, so what you're suggesting is unfortunately unsound.
For example, you could have some function:
def f(x: Foo) -> str:
return x.factory()
Because of the way subtyping relationships work in mypy, it's valid to pass a Bar anywhere you need a Foo (see Guido's comment about the Liskov Substitution Principle above). But if Bar.factory returns an int, passing a Bar to f is clearly invalid.
At the moment I'm using mypy to validate stub files I am generating. I'm not sure if this error is saying that, in an otherwise correct stub file, mypy can't do its job properly? Or is it saying that the stub file is incorrect no matter what tool is using it?
Phil, does the real code where you hit this happen to use @staticmethod? Because that could be considered a different case.
Alternatively (if you don't like the # type: ignore solution) it might be possible to refactor the classes so that Bar is not a subclass of Foo? (What they do have in common could be moved to a base class of Foo.)
Regarding whether the stub files are valid or not, I think that a stub file containing the 6 lines of code you showed us should be considered objectively invalid. Whether you can read that in the letter of PEP 484 is a different issue.
Let me try to clarify some more. Suppose we have a file a.py like this:
from defs import Foo
def buzz(f: Foo) -> str:
return f.factory()
And suppose we have b.py:
from defs import Bar
from a import buzz
b = Bar()
buzz(b)
There is a type error here, since at run time buzz(b) will return an int, despite buzz's signature telling us that it returns a str. Where should the type error be reported? Bar is a subclass of Foo, so the call buzz(b) is not at fault, so b.py type-checks (and if the buzz() call is used, it will be treated as returning a string, since that's what a.py says it returns). Also, Foo.factory() returns a str, so a.py is not at fault either. Therefore the only other place that can be faulted is defs.py[i], and that's what you're seeing. I don't think that's unreasonable or unexpected.
Perhaps the best solution really is to make it so that Bar is not a subclass of Foo. Is that reasonable?
On 24 Feb 2016, at 10:53 pm, Guido van Rossum [email protected] wrote:
Phil, does the real code where you hit this happen to use @staticmethod? Because that could be considered a different case.
In some cases but not all.
Alternatively (if you don't like the # type: ignore solution) it might be possible to refactor the classes so that Bar is not a subclass of Foo? (What they do have in common could be moved to a base class of Foo.)
I have no control over the API I am trying to describe.
I don't like #type because (I'm guessing) other tools (like an IDE using the stub files to create auto-completion lists) would also ignore it.
Regarding whether the stub files are valid or not, I think that a stub file containing the 6 lines of code you showed us should be considered objectively invalid. Whether you can read that in the letter of PEP 484 is a different issue.
The following is from my real stub files...
class QPixmap(QPaintDevice):
def swap(self, other: 'QPixmap') -> None: ...
class QBitmap(QPixmap):
def swap(self, other: 'QBitmap') -> None: ...
...as far I understand PEP 484 that is the correct way to describe the API in a stub file. If you are saying that it is not possible to create a PEP 484-compliant stub file for this API the I would suggest that PEP 484 isn't much use for real applications.
Even if the stub file is PEP 484-compliant, but the API makes it difficult for mypy to do the type checking it wants to, then it should still handle it more gracefully than it does at the moment. The stub files are for PyQt so, as it stands, mypy cannot be used on PyQt applications. I don't see PyQt as being special in this regard, I imagine that there could be many 3rd-party libraries that break these rules, particularly if they wrap C++ libraries.
I have no control over the API I am trying to describe.
Understood. Do you have control over the stub generating tool?
I don't like #type because (I'm guessing) other tools (like an IDE using the stub files to create auto-completion lists) would also ignore it.
Well, PyCharm is sprinting to support all of PEP 484, and their latest (5.1-EAP) even supports the Python 2.7 signature type comments (e.g. # type: (int, int) -> float) that I added to the PEP recently. I've also talked to authors of both flake8 and pyline and they are also planning to support # type: comments.
Of course I don't know what your favorite IDE is or what its support plan is for any of this.
Regarding this example:
class QPixmap(QPaintDevice):
def swap(self, other: 'QPixmap') -> None: ...
class QBitmap(QPixmap):
def swap(self, other: 'QBitmap') -> None: ...
That's a slightly different situation again. Here it seems you have arguments that vary covariantly, which goes against the "Liskov" model that I explained in an earlier comment here. Interestingly, while most languages (and PEP 484*) declare this invalid, in Eiffel this is actually valid (and they insert a runtime check in case a QBitmap down-casted to a QPixmap is called with a QPixmap argument, since the compiler doesn't catch that error).
...I would suggest that PEP 484 isn't much use for real applications.
That came across unnecessarily harsh (and doesn't match my experience at Dropbox, where we are successfully introducing it to what I consider very much a "real application".)
I would like to propose two ways forward -- a short-term way and a long-term approach. In the short term, I think you can solve this by adding a few well-aimed # type: ignore comments to your stubs, or perhaps by using Any in some places. (I hesitate to recommend severing the inheritance between QPixmap and QBitmap, since I imagine that would cause more problems than it solves.)
In the longer term, I think we can add a directive to PEP 484 to accommodate your use case. For example, if we were to let you write
from typing import covariant_args
class QPixmap... # unchangd
class QBitmap(QPixmap):
@covariant_args
def swap(self, other: 'QBitmap') -> None: ...
That should allow you to choose the Eiffel model over the Liskov model for a specific method. Or if you have use cases where the arguments are just entirely different we could come up with some other decorator name.
Side note: as a temporary fix, you can set the retuen type of both factory methods to Union[str, int].
On 26 Feb 2016, at 9:36 pm, Guido van Rossum [email protected] wrote:
I have no control over the API I am trying to describe.
Understood. Do you have control over the stub generating tool?
Complete control - it's what I am trying to test.
I don't like #type because (I'm guessing) other tools (like an IDE using the stub files to create auto-completion lists) would also ignore it.
Well, PyCharm is sprinting to support all of PEP 484, and their latest (5.1-EAP) even supports the Python 2.7 signature type comments (e.g. # type: (int, int) -> float) that I added to the PEP recently. I've also talked to authors of both flake8 and pyline and they are also planning to support # type: comments.
I think you misunderstood. I'm not concerned that an IDE would not recognise the ignore directive - quite the opposite. I'm concerned that it would recognise it and omit the method from generated auto-completion lists.
Of course I don't know what your favorite IDE is or what its support plan is for any of this.
Regarding this example:
class QPixmap(QPaintDevice
):def swap(self, other: 'QPixmap') -> None: ...
class QBitmap(QPixmap
):def swap(self, other: 'QBitmap') -> None: ...
That's a slightly different situation again. Here it seems you have arguments that vary covariantly, which goes against the "Liskov" model that I explained in an earlier comment here. Interestingly, while most languages (and PEP 484*) declare this invalid, in Eiffel this is actually valid (and they insert a runtime check in case a QBitmap down-casted to a QPixmap is called with a QPixmap argument, since the compiler doesn't catch that error)....I would suggest that PEP 484 isn't much use for real applications.
That came across unnecessarily harsh (and doesn't match my experience at Dropbox, where we are successfully introducing it to what I consider very much a "real application".)
Apologies - wait till you get to the PyQt parts of your application :)
I would like to propose two ways forward -- a short-term way and a long-term approach. In the short term, I think you can solve this by adding a few well-aimed # type: ignore comments to your stubs, or perhaps by using Any in some places. (I hesitate to recommend severing the inheritance between QPixmap and QBitmap, since I imagine that would cause more problems than it solves.)
Do you have a contact on the PyCharm development so I can ask them what works best for them. In this context I'm a supplier of stub files, not a user.
In the longer term, I think we can add a directive to PEP 484 to accommodate your use case. For example, if we were to let you write
from typing import
covariant_argsclass QPixmap... # unchangd
class QBitmap(QPixmap
):@covariant_args
def swap(self, other: 'QBitmap') -> None: ...
That should allow you to choose the Eiffel model over the Liskov model for a specific method. Or if you have use cases where the arguments are just entirely different we could come up with some other decorator name.
My problem with that is that it seems too specific - there may be lots of other ways that the stub files are invalid that I haven't discovered yet.
Assuming that stub files will be used for more than mypy-like type checking (eg. auto-completion lists) then having something @ignore(reason) with a standard set of reasons might work. Different tools could then selectively ignore a signature.
• While PEP 484 may currently not explicitly describe the exact rules for how subclass methods are allowed to vary compared to the base class method they override, its mentions of variance (and Python's general insistence on the Liskov world-view) make it clear that it sees the world this way. Maybe we could add words to the PEP to clarify this, but I don't the intention is actually unclear (unless we were to try to do a constitutional-lawyer or Bible/Torah expert level attempt to reinterpret an immutable document to changing times :-).
From reading the PEP I don't get any feeling that it's only intended to cover a conforming sub-set of Python code. I read it and think here is a specification for describing any Python API in a standard way.
My contact at PyCharm is Andrey Vlasovskikh. You can probably reach him by calling out to him in the tracker here: https://github.com/python/typing/issues (he's subscribed to all issues there) or in the PyCharm tracker at youtrack.jetbrains.com. I'll try to address the rest later.
From reading the PEP I don't get any feeling that it's only intended to cover a conforming sub-set of Python code. I read it and think here is a specification for describing any Python API in a standard way.
It's not possible to describe all possible valid Python API's statically, except _perhaps_ with an exceedingly complex type system. For example, you could have some function:
def f(x: int) -> ???:
if x < 0: return 0
else: return "hello"
and call it like this:
print(f(5) + " world")
This program will execute without errors, but the only way to type it properly is with dependent types, which are rather difficult to understand/work with.
On 27 Feb 2016, at 12:39 am, David Fisher [email protected] wrote:
From reading the PEP I don't get any feeling that it's only intended to cover a conforming sub-set of Python code. I read it and think here is a specification for describing any Python API in a standard way.
It's not possible to describe all possible valid Python API's statically, except perhaps with an exceedingly complex type system. For example, you could have some function:
def f(x: int) -> ???
:if x < 0: return 0
else: return "hello"
and call it like this:print(f(5) + " world")
This program will execute without errors, but the only way to type it properly is with dependent types, which are rather difficult to understand/work with.
Understood - if the context is restricted to type checking - but there may be other contexts (eg. generating boilerplate documentation) where ??? is Union[int, str] is a useful piece of information.
There's a distinction here that can be useful and relevant which is subclassing vs subtyping. By subclass I mean "making A a subclass of B gives A all the code from B unless overridden". By subtype I mean Liskov substitution "A is a subtype of B when I can use an A to anything declared of type B". In many languages, both things happen at the same type. One of the nice things of Python and dynamic languages is that you can have subtyping with no subclassing (i.e., duck-typing: if you implement the same interface you can implicitly have Liskov substitution).
Something that has been done (also in Eiffel, but unrelated to the covariance scenario above) is having subclassing without subtyping. You can have a subclass to get all the method definitions from the parent, but the new class is not a subtype of the other. In that case you're applying inheritance for code reuse, not for Liskov substitution. They call it "implementation inheritance"
In this scenario, the original code could be accepted, but QBitmap wouldn't be a class of QPixmap. So the following happens:
p1 = QPixmap(); p2 = QPixmap()
b1 = QBitmap(); b2 = QBitmap()
lp = [] # type: List[QPixmap]
lb = [] # type: List[QBitmap]
lp.append(p1) # this is ok
lp.append(b1) # this is rejected, a bitmap is not a pixmap even with the subclassing
lb.append(b1) # this is ok
lp[0].swap(p2) # OK
lb[0].swap(p2) # wrong, swap of a pixmap with a bitmap
The main point is that I'm quite sure that there are probably many pieces of Python around that use subclassing but where Liskov substitution was not required nor intended, so probably it shouldn't be demanded.
If you agree with this, probably it makes sense to force the programmers to make explicit that they actually want implementation inheritance. For example
class QBitmap(Implementation[QPixmap]):
def swap(self, other: 'QBitmap') -> None: ...
once you make explicit that you inherit implementation you can change types in any direction (covariant, contravariant, change number of arguments, etc)
That's a good way to frame it. I would recommend a class decorator to indicate this. We should move this discussion to https://github.com/python/typing as a proposal to add such a class decorator to PEP 484 and typing.py.
OK, I'll create an issue there (I didn't see one)
I think that implementation inheritance can be type checked by re-type-checking the bodies of all base class methods as if they were written in the subclass (after translating type variables suitably if the base class is generic).
@JukkaL that should be right (and that's actually how they were implemented in the Eiffel compiler I used to work in)
I added python/typing#241 to continue the discussion
I noticed this error is occurring with static methods as well:
class Foo:
@staticmethod
def create(arg: int) -> Foo:
return Foo(...)
class Bar(Foo):
@staticmethod
def create(arg: str) -> Bar:
return Bar(...)
Should it be reported in this case?
That's still a Liskov violation, because you can call static methods via the instance, e.g.
def f(a: Foo):
a.create(42)
f(Bar())
In this particular case though, you can probably fix the type errors by making the affected methods classmethods of the form
@classmethod
def create(cls: Type[T], arg: int) -> T: ...
where T is a type variable.
Consistency versus compatibility with supertype (PEP-483):
A type t1 is consistent with a type t2 if t1 is a subtype of t2. (But not the other way around.)
Unions whose components are all subtypes of t1 etc. are subtypes of this. Example: Union[int, str] is a subtype of Union[int, float, str].
Yet:
class FormDict:
def setval(self, key: Union[str, int], val: Any): ...
class WidgetAttribute(FormDict):
def setval(self, key: Union[str], val: Any): ...
Generates incompatible with supertype, even though according to the above, they are consistent. To me, consistent with and compatible are synonyms, so while Liskov is mentioned in this thread, PEP 483 contradicts it or at best, doesn't specify that covariance cannot be applied to function arguments and return types. In fact - there are examples that suggest it is possible, but so far, haven't been able to come up with a declaration that works for subclassing.
@melvyn-sopacua It looks like you don't like something, but I don't understand what exactly. Maybe you have missed the section in PEP 483 that explains that callables behave _contravariant_ in the argument types (with examples). You might also want to read Callable example here.
while Liskov is mentioned in this thread, PEP 483 contradicts it or at best
This is quite serious accusation, could you please elaborate more?
You can use a # type: ignore on a method override to stop mypy complaining about it, if you don't care about the Liskov violation.
Looking into it more, it's quite confusing:
class Field(Generic[T_co]):
def to_python(self, value: Union[int, float]) -> Any: ...
class IntegerField(Field):
def to_python(self, value: Union[int, float]) -> int: ...
class FloatField(IntegerField):
def to_python(self, value: Union[float]) -> int: ...
class IntegerOnlyField(Field):
def to_python(self, value: Union[int]) -> int: ...
This trips up only on the IntegerOnlyField. The fact that I don't get why, is that none of this is covered in PEP-483.
This is quite serious accusation, could you please elaborate more?
class Box(Generic[T_co]): # this type is declared covariant
def __init__(self, content: T_co) -> None:
That example tells me "one can declare arguments to be covariant". Which says to me "If I put a union there, then all unions that intersect with parent are going to be compatible with parent". This is analog with "If a method accepts an int or float argument any subclasses that override the method can either accept an int, a float or both". Which is a quite natural way of "specializing".
I'm sure that's with my interpretation, but it's not clear from what I've been reading.
(I'm leaving the fact that FloatField's to_python cannot return a float out of scope for now)
@melvyn-sopacua
That example tells me "one can declare arguments to be covariant".
__init__ and __new__ are special, mypy allows to override them literary as you wish, no restrictions. (There is a reason for this, otherwise how would you put something in a covariant container, plus this (i.e. incompatible __init__) is very common in practice).
EDIT: mentioned practical aspect.
Understood.
Still, I'm skimming through PEP 483 again and everything dealing with inheritance deals only with the effect on the declared classes' type, not on how their methods should behave or be declared.
But The wording here is confusing:
Unions whose components are all subtypes of t1 etc. are subtypes of this. Example: Union[int, str] is a subtype of Union[int, float, str].
The second cannot be true, since it would infer that int and str are subtypes of each other.
The first cannot be true, cause then IntegerOnlyField in my example should not have a conflict.
I feel like I'm missing something fundamental that is obvious to you here.
The wording in PEP 483 is poor; what you infer from the example is right.
Feel free to send a PR with updates to PEP 483 (but try not to fix everything at once -- that's a recipe for death in review).
@gvanrossum
what you infer from the example is right.
But it is not the whole story. The expanded version of that quoted statement is:
If U1 = Union[t1, ..., tn] and U2 = Union[s1, ..., sn] and _every_ ti is a subtype of _at least one_ of sj then U1 is a subtype of U2. For example:
class A: ...
class B: ...
class C(A): ...
class D(B): ...
class E(A, B): ...
x: Union[A, B]
y: Union[C, D, E]
x = y # OK
Union types behave just as sets
@ilevkivskyi but that only applies if none of y is in x. Then consistency is expanded to include subtypes of elements in x (covariance).
Which means, covariance isn't part of the problem in the case of IntegerOnlyField failing, since it fails the set member comparison: Union[int, float] > Union[float].
Would this be accurate:
def union_compare(u1, u2):
u1 = set(u1)
u2 = set(u2)
if u1 >= u2:
return True
if u1 < u2:
return False
for first in u1:
first = first or type(first)
for second in u2:
second = second or type(second)
if issubclass(second, first):
return True
return False
print(repr(union_compare([int, float], [float]))) # True
print(repr(union_compare([int, float], [bool]))) # True
print(repr(union_compare([int, float], [float, int]))) # True
print(repr(union_compare([float], [int, float]))) # False
print(repr(union_compare([int, float], [None]))) # False
Edit: superset comparison should include equal set
@melvyn-sopacua
but that only applies if none of y is in x
"That" (I suppose my extended explanation) applies always, since X is a subtype of X for every X (and every type is consistent with itself).
Which means, covariance isn't part of the problem in the case of IntegerOnlyField failing
I already explained that the unions are not the problem for that example, but contravariance
of callables in arguments, this simpler example fails as well:
class X: ...
class Y(X): ...
class B:
def method(self, arg: X) -> None: ...
class C(B):
def method(self, arg: Y) -> None: ... # Incompatible with supertype ...
while this works:
class B:
def method(self, arg: Y) -> None: ...
class C(B):
def method(self, arg: X) -> None: ... # OK
I even sent you some links above, but it looks like you didn't read them.
I did read them, and totally get the example above - but I don't see how they apply in the case of unions.
Given that:
>>> issubclass(float, int)
False
>>> issubclass(int, float)
False
We can rule out any variance rules of the members, or issubclass isn't the correct test.
If issubclass is the correct test, then I do not understand the difference in Union[int] (succeeds) and Union[float] (fails) in comparison with Union[int, float].
If is issubclass is not the correct test and float is a subtype of int, then what is different is that Union[int, float] contains interfaces that are not in implemented in Union[float].
And this is the fundamental part I was missing. The initial definition of subtype in PEP483 supports this:
But then my inference that subset comparison equals subtype contradicts it:
only works cause everything float implements is already in int, not because int and str are in int, float, str.
If this is correct, then I know how to avoid the confusion in the union section of PEP 483.
Mypy treats int as a subtype of float, even though int is not a subclass of float. This is an exception to how things normally work.
Thank you all!
Concluding:
float.from_bytes() fails and is unsafe, float passes contra-variance test because of special treatment in mypy and is assumed to have from_bytes from int.Contra-variant behavior of method arguments with Union is best illustrated:
class Field:
def to_python(self, value: Union[str, int]): ...
class MultiValueField:
def to_python(self, value: Union[str, int, List]): ... # OK
Even though it feels natural that the base class should be less specific, one must actually think that the specialized class is capable of handling more.
__init__ and __new__are special, mypy allows to override them literary as you wish, no restrictions. (There is a reason for this, otherwise how would you put something in a covariant container, plus this (i.e. incompatible__init__) is very common in practice).
Is there any way currently to extend this property to other methods on demand? The repository i'm trying to type has a pattern of @classmethods called create which are used as alternative constructors - these aren't intended to be related to the inheritance hierarchy.
At the moment I can bypass via type: ignore - but I don't really want to ignore them, I just want to tell mypy not to treat these class methods as related.
@glenjamin
but I don't really want to ignore them, I just want to tell mypy not to treat these class methods as related.
type: ignore doesn't ignore types (or anything) it ignores type errors. Essentially, it just silences all errors at the line where it is placed, but has literally _no_ other effects.
type: ignoredoesn't ignore types (or anything) it ignores type errors. Essentially, it just silences all errors at the line where it is placed, but has literally no other effects.
So if i silence an error at the function definition, i'll still get type errors if I use that function in a way that doesn't respect the definition elsewhere?
i'll still get type errors if I use that function in a way that doesn't respect the definition elsewhere?
Yes.
I don't think that there is anything actionable here. It's now possible to use # type: ignore[override] to specifically silence the incompatible override error (and in a way that is hopefully less confusing than # type: ignore).
Most helpful comment
Yes.