Coder Social home page Coder Social logo

Surprising issubclass result about pytypes HOT 27 CLOSED

stewori avatar stewori commented on June 19, 2024
Surprising issubclass result

from pytypes.

Comments (27)

mitar avatar mitar commented on June 19, 2024

Now, the following return true:

T = typing.TypeVar('T', covariant=True)
L = typing.List[T]
type_util._issubclass(L[float], L[typing.Any])

But this is false again:

type_util._issubclass(L[float], L)

This is surprising in some ways. First, L should really be the same as L[typing.Any]. That's how it is defined in Python's typing.

But the other thing is that covariance should matter when we are talking about return types from a function, but for a stand-alone type comparison it should not, no?

from pytypes.

Stewori avatar Stewori commented on June 19, 2024

List is unfortunately invariant as of PEP 484, which means that nothing is a subclass of List[Any] but List[Any] itself. Reason for this is that list is writable. I suppose Sequence[Any] would do what you want.

T = typing.TypeVar('T', covariant=True)
L = typing.List[T]
type_util._issubclass(L[float], L[typing.Any])

Using a covariant typevar as value for an invariant type parameter seems invalid to me. This is currently undefined behavior, but I suppose pytypes should throw an error in this case. That said, it seems to accept it, but you're right the observed behaviour is not very pleasant.

type_util._issubclass(L[float], L)

should behave like the Any version. So labelling this as a bug...

from pytypes.

mitar avatar mitar commented on June 19, 2024

This is currently undefined behavior, but I suppose pytypes should throw an error in this case.

Please don't. :-) This is a workaround I am currently using. The issue with Sequence is that it is an abstract type, you cannot really create it. Maybe we should be using typing.Tuple there instead, but currently we are using List which we in practice use as immutable. So this works for us, but it is not enforced by type checking, true.

Moreover, one other reason why this should not be an error is because you might want to signal that a particular function is not changing the List and that it can accept then types in a covariant manner. This is in fact an example from PEP 484, exactly about List. So it seems that one should be able to declare that a list acts covariantly, even if it is mutable.

from pytypes.

Stewori avatar Stewori commented on June 19, 2024

Please don't. :-)

No worries, I want pytypes to be convenient. It is already less strict than PEP 484 in case of mutable maps, see pytypes.covariant_Mapping

I still think it's fine to use Seqence as type annotation even though no corresponding type exists in Python. AFAIK it is the official way to denote that something accepts lists and tuples and other sequence-like objects.

This is in fact an example from PEP 484, exactly about List.

Hmmm I don't find an example there where they simply use List[T_co]. As I understood it one has to define a new subclass of Generic: class ImmutableList(Generic[T_co]): ...

Anyway, for me it seems okay to let pytypes accept your version as well.

from pytypes.

mitar avatar mitar commented on June 19, 2024

I was thinking about this section:

Consider a class Employee with a subclass Manager. Now suppose we have a function with an argument annotated with List[Employee]. Should we be allowed to call this function with a variable of type List[Manager] as its argument? Many people would answer "yes, of course" without even considering the consequences. But unless we know more about the function, a type checker should reject such a call: the function might append an Employee instance to the list, which would violate the variable's type in the caller.

It turns out such an argument acts contravariantly, whereas the intuitive answer (which is correct in case the function doesn't mutate its argument!) requires the argument to act covariantly.

So if function does not mutate the argument, argument can act covariantly.

But you are right that in 483 they discuss how one should create an immutable container and how:

Note, that although the variance is defined via type variables, it is not a property of type variables, but a property of generic types.

from pytypes.

mitar avatar mitar commented on June 19, 2024

The following works though:

import typing
from pytypes import type_util

T = typing.TypeVar('T', covariant=True)

class L(typing.List[T]):
    pass

type_util._issubclass(L[float], L)

from pytypes.

Stewori avatar Stewori commented on June 19, 2024

Yes, that's exactly the variant I'd consider valid and I think I took care to support. There are also tests for this.

from pytypes.

mitar avatar mitar commented on June 19, 2024

Playing with this a bit more.

import typing
from pytypes import type_util

class Foo:
    pass

type_util._issubclass(typing.Tuple[Foo], typing.Tuple[object, ...])  # False?
type_util._issubclass(typing.Tuple[Foo], typing.Tuple[typing.Any, ...])  # False?

I would assume those to be true?

The issue comes from type_util._isinstance((Foo(),), typing.Tuple[typing.Any, ...]), this should probably be true. Not to even mention type_util._isinstance((Foo(),), typing.Tuple[Foo, ...]).

from pytypes.

Stewori avatar Stewori commented on June 19, 2024

Maybe the ellipsis notation for tuple is not yet properly supported. (Maybe not at all, I'd have to look into the code...)

from pytypes.

Stewori avatar Stewori commented on June 19, 2024

It would be very helpful if you could write your experiments in a form that can be used as a test.
We could ideally add it to the test suite, e.g. with something like @skip("Not yet supported, see issue ..."). Lack of tests, also for already fixed issues, is a significant source of delay for the next release.

from pytypes.

mitar avatar mitar commented on June 19, 2024

Oh, sorry. Those examples above are self-contained. You can just copy-paste them into a Python interpreter and they should exercise the error. I thought this is enough to wrap them into tests. What would you prefer instead? A PR adding this as a test? Or some other syntax here in the issue?

from pytypes.

Stewori avatar Stewori commented on June 19, 2024

Yes, ideally a PR. Turning experiments into test often involves some subtle additional work, yet usually rather trivial. E.g. finding the right place in the messy test suite (yes we need a better test system,help welcome), choosing the right name, add eventually both a Python 2 and 3 version. While turning it into a test one sometimes finds that some additional cases should be covered as well.
Some help on this front would greatly help me to focus on solutions and would speed up the next release.

from pytypes.

Stewori avatar Stewori commented on June 19, 2024

I just noticed that is_subtype(Foo, object) is false in Python 2, unless we explicitly declare class Foo(object): pass. I guess this behavior reflects ordinary behavior of issubclass, which is fine for me. However this is one example of the pitfalls one might encounter in test-writing.

from pytypes.

Stewori avatar Stewori commented on June 19, 2024

I think the ellipsis-related issues are fixed now as of 774e4be. Can you confirm? Still, a tests-PR would be nice.

from pytypes.

mitar avatar mitar commented on June 19, 2024

So I think one issue here is that Python standard issubclass works here differently:

>>> type_util._issubclass(typing.List[float], typing.List)
False
>>> issubclass(typing.List[float], typing.List)
True

from pytypes.

Stewori avatar Stewori commented on June 19, 2024

Hmm, this is due to the invariance of List's type parameter. If parmeterless List is treated as List[Any] this behavior is okay. Wanted to point out there are options:

check_unbound_types : bool
	If true, treat missing parameters as unknown.
	Default: True
	Tells pytypes to actually attempt typechecking of unbound types, e.g
	things like is_subtype(List[Any], list).
	If false such checks are prohibited.
	If true, missing parameters are treated as unknown, which in turn is
	treated according to strict_unknown_check flag.

strict_unknown_check : bool
	Controls the meaning of unknown parameters.
	Default: False
	If false, treat unknown parameters somewhat like Any.
	If true (i.e. strict mode), treat unknown parameters
	somewhat like 'nothing', because no assumptions can be made.

However these can only make checking even stricter, e.g. disallow parameterless .

from pytypes.

mitar avatar mitar commented on June 19, 2024

Yes, I understand where is coming from. It is just surprising. I was hoping to just replace all isinstance with pytypes. It seems I might want to do or between both. :-) If any says true, then it is an instance. :-)

Not sure what could be changed here to make this different, while keeping the proper type checking. It is just in practice surprising when you just want to see that something is a subclass or instance. Maybe we could differentiate between explicit List[Any] and List? To me that would be the best. But it is against the PEP, I think.

from pytypes.

Stewori avatar Stewori commented on June 19, 2024

Hmm, I actually differentiated between List[Any] and List, but not the way you suggest (i.e. treat it like List[unknown]). Partly, all this is why I aliased _isinstance with is_of_type for public API - to underline that it is not fully equal to isinstance. (Btw. replacing all isinstance is not a good idea performance-wise, as pytypes is much slower than plain isinstance e.g. because it scans for stubfiles and type comments). Then there is also list. I think that should be working like List. Maybe we could add one global parameter to tell pytypes to handle List specially as covariant, much like pytypes.covariant_Mapping works for dictionaries (and other mappings). So far I avoided this because using typing.Sequence solves most issues related with this.

from pytypes.

Stewori avatar Stewori commented on June 19, 2024

IIRC regarding list and other collection types, PEP484 defines the behavior like check_unbound_types = False would yield.

from pytypes.

Stewori avatar Stewori commented on June 19, 2024

Also see #34 (comment).
I admit that deep_type inferring List[float] e.g. from [1.2, 3.4, 2.5] in invariant sense is maybe too restrictive. List being invariant makes sense for static typechecking, but not so much for runtime typechecking. Maybe the semantic of Empty could be extended to cover this case (given that it is already solving a related issue). (Along with renaming it to Incomplete or Sampled or so. Then it is a marker that more types are fine for writing.

from pytypes.

mitar avatar mitar commented on June 19, 2024

A very good point and interesting point about runtime vs. static time checking. At runtime you can really know precisely what the value is. So some constraints might not be reasonable. Thanks for this insight.

from pytypes.

Stewori avatar Stewori commented on June 19, 2024

Skimmed through PEP 484 again and found this note:

Note: Dict, DefaultDict, List, Set and FrozenSet are mainly useful for annotating return values. For arguments, prefer the abstract collection types defined below, e.g. Mapping, Sequence or AbstractSet.

However this does not answer how to handle the case that a function really intends typesafe write-access to a list object passed in as parameter. Maybe the typechecked-decorator should offer some feature to configure this per-function additional to a global flag.

from pytypes.

mitar avatar mitar commented on June 19, 2024

We got hit again by this. Really hard to debug.

Hmm, this is due to the invariance of List's type parameter. If parmeterless List is treated as List[Any] this behavior is okay. Wanted to point out there are options:

There are not options to configure that type_util._issubclass(typing.List[float], typing.List) should return True, no?

from pytypes.

Stewori avatar Stewori commented on June 19, 2024

The only viable option at this moment is to type it using Sequence. If you need a writable type maybe Union[List, Sequence] could work...?
I cannot tell when I can provide a proper fix for this (PR welcome).

from pytypes.

mitar avatar mitar commented on June 19, 2024

What exactly do you think PR should do? How you imagine a change to support this?

from pytypes.

Stewori avatar Stewori commented on June 19, 2024

You could implement a special case for list in similar fashion as is done for dict already.
Better solution and proper solution for long term would be to widen the use of empty to cover list and dict. In that case empty should be renamed to sampled and serve as a marker for types that were guessed via deep_type.

from pytypes.

Stewori avatar Stewori commented on June 19, 2024

The List being invariant issue is now tracked explicitly in #76. So I'll close this one in favor of #76.

from pytypes.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.