python / typeshed

Collection of library stubs for Python, with static types
Other
4.37k stars 1.75k forks source link

Problems with the new ctypes stubs #1983

Closed JelleZijlstra closed 6 years ago

JelleZijlstra commented 6 years ago

From @ilevkivskyi in python/mypy#4786:

It looks like the ctypes stubs cause several problems in some internal code bases. This is mostly due to mypy's own limitation in treatment of platform checks. But it looks like there may be some missing or problematic definitions:

CArgObject missing wintypes missing It looks like memmove can also accept str and _cparam There are lots of errors like Unsupported operand types for ("Type[Array[]]" and "int"), so I suppose Array has a metaclass that implements mul/rmul like _CDataMeta. Signatures of _CDataMeta.mul and _CDataMeta.rmul are wrong, they contain unbound type variable resulting in . Also it looks like mypy has problems with type inference for types, so Array.init is currently problematic. The stub looks good overall, so I don't want to revert it, but it needs some polishing before it gets into the next release.


@dgelessus would you be interested in fixing these issues?

dgelessus commented 6 years ago

Thanks for the feedback @JelleZijlstra @ilevkivskyi! My own testing of the stubs wasn't very extensive - the codebase I'm working on has lots of dynamic parts that can't be typechecked (it's an FFI basically) so I didn't have anything to properly test the stubs against. I'm happy to hear about any "real world experiences" 😃

  • CArgObject missing

That's the type returned from byref, right? In the stub it's called _cparam, because in its repr it calls itself cparam.

  • wintypes missing

Right, I'm mainly a Mac user, so I didn't have a quick way to check out what that module does. Will have a look at that. (I have some Windowses too, I just don't use them for development normally.)

Also, I thought that if a stub is missing for a module, mypy would fall back to looking at the source file instead. Is that different if the parent module has a stub but the submodule doesn't? Or is there a different reason why mypy isn't recognizing wintypes?

  • It looks like memmove can also accept str and _cparam

Right, memmove is actually a C function pointer, so it supports all the usual argument conversions. Same goes for memset. (I'm guessing by str you mean bytes rather than Text?)

  • There are lots of errors like Unsupported operand types for * ("Type[Array[<nothing>]]" and "int"), so I suppose Array has a metaclass that implements __mul__/__rmul__ like _CDataMeta.

Huh, Array inherits from _CData, so it should inherit the metaclass as well. Not sure what's going on there. Could it have to do with the unbound type variable perhaps?

  • Signatures of _CDataMeta.__mul__ and _CDataMeta.__rmul__ are wrong, they contain unbound type variable resulting in <nothing>.

Would Type[Array[Any]] be more appropriate here? It's not possible to determine the array element type properly, as far as I can tell, see comment.

  • Also it looks like mypy has problems with type inference for * types, so Array.__init__ is currently problematic.

I'm not sure what you mean with "* types", sorry... Do you mean the array type returned by something like c_int * 8? The type for that can't be determined properly I think, see above.

Thanks again for the feedback, will try to fix these issues soonâ„¢.

JelleZijlstra commented 6 years ago

Thanks! To be clear @ilevkivskyi came up with the list of issues, not me. I am not familiar with these issues, but can help look into it more.

ilevkivskyi commented 6 years ago

@dgelessus Here are some clarifications:

CArgObject missing

That's the type returned from byref, right? In the stub it's called _cparam, because in its repr it calls itself cparam.

I (and most other users of this stub) don't really care what is its repr, if this type is accessible at runtime under attribute ctypes.CArgObject, then this should be reflected in the stub.

Huh, Array inherits from _CData, so it should inherit the metaclass as well. Not sure what's going on there. Could it have to do with the unbound type variable perhaps?

Yes, this may be caused by the wrong type of __(r)mul__, please fix this. Or maybe because of a metaclass conflict (Sized is declared in typeshed as an instance of ABCMeta), could you please remove Sized from base classes, and instead add a __len__ method with an appropriate signature? (This will work since Sized is a protocol).

Also it looks like mypy has problems with type inference for * types, so Array.init is currently problematic.

I'm not sure what you mean with " types", sorry... Do you mean the array type returned by something like c_int 8? The type for that can't be determined properly I think, see above.

No, I think there may be some bug in mypy, I have ideas for how to change the signature, but let us postpone this until other things are fixed.

Btw, thanks for working on this, I hope you will fix this time for mypy 0.590!

dgelessus commented 6 years ago

I (and most other users of this stub) don't really care what is its repr, if this type is accessible at runtime under attribute ctypes.CArgObject, then this should be reflected in the stub.

Hm, for me that attribute doesn't exist... What Python version and OS are you on? ctypes.CArgObject exists on none of my Python versions (all installed with the official OS X installers from python.org).

$ for ver in 2.5 2.6 2.7 3.3 3.4 3.5 3.6
> do
> /Library/Frameworks/Python.framework/Versions/${ver}/bin/python${ver} -c "import ctypes, sys; print(sys.version); print('CArgObject' in dir(ctypes))"
> done
2.5.4 (r254:67917, Dec 23 2008, 14:57:27) 
[GCC 4.0.1 (Apple Computer, Inc. build 5363)]
False
2.6.6 (r266:84374, Aug 31 2010, 11:00:51) 
[GCC 4.0.1 (Apple Inc. build 5493)]
False
2.7.13 (v2.7.13:a06454b1afa1, Dec 17 2016, 12:39:47) 
[GCC 4.2.1 (Apple Inc. build 5666) (dot 3)]
False
3.3.5 (v3.3.5:62cf4e77f785, Mar  9 2014, 01:12:57) 
[GCC 4.2.1 (Apple Inc. build 5666) (dot 3)]
False
3.4.4 (v3.4.4:737efcadf5a6, Dec 19 2015, 20:38:52) 
[GCC 4.2.1 (Apple Inc. build 5666) (dot 3)]
False
3.5.4 (v3.5.4:3f56838976, Aug  7 2017, 12:56:33) 
[GCC 4.2.1 (Apple Inc. build 5666) (dot 3)]
False
3.6.4 (v3.6.4:d48ecebad5, Dec 18 2017, 21:07:28) 
[GCC 4.2.1 (Apple Inc. build 5666) (dot 3)]
False

Yes, this may be caused by the wrong type of __(r)mul__, please fix this. Or maybe because of a metaclass conflict (Sized is declared in typeshed as an instance of ABCMeta), could you please remove Sized from base classes, and instead add a __len__ method with an appropriate signature? (This will work since Sized is a protocol).

Tested with the current state of #1983 #1986 (where the return type has been fixed), removing Sized doesn't help. Removing Generic doesn't help either (after fixing all the other errors about the missing type parameter). I'm guessing it's a more general problem, it happens with _Pointer as well. Here's my test code:

import ctypes

intarr4 = ctypes.c_int * 4
intarr4arr4 = intarr4 * 4  # error: Unsupported operand types for * ("Type[Array[Any]]" and "int")
intptrarr4 = ctypes.POINTER(ctypes.c_int) * 4  # error: Unsupported operand types for * ("Type[_Pointer[c_int]]" and "int")

Btw, thanks for working on this, I hope you will fix this time for mypy 0.590!

Will try 😃 Is there any release date planned for that version yet?

ilevkivskyi commented 6 years ago

Hm, for me that attribute doesn't exist...

Interesting, OK, then leave it out. Maybe it is a custom build or something.

I'm guessing it's a more general problem

Could you please find a minimal self-contained repro and open an issue on mypy tracker? It would be also helpful if you could find some simple repro for the bug when one calls Array(*some_list) and inferred types of the result contains <nothing> or an error is reported at the call site.

Is there any release date planned for that version yet?

Around April 14.

dgelessus commented 6 years ago

Interesting, OK, then leave it out. Maybe it is a custom build or something.

FWIW, the CArgObject name can be seen in Python, but only when you look at type(byref(...)) by hand. Other than that, the CArgObject name is used only in the CPython source code for the type. In that respect, _CArgObject might be a better name than _cparam. I wouldn't make it public though, since apparently the attribute doesn't always exist. (If we can figure out with which Python version and/or OS the attribute exists, we could conditionally make it public of course.)

Could you please find a minimal self-contained repro and open an issue on mypy tracker?

Done: python/mypy#4789

It would be also helpful if you could find some simple repro for the bug when one calls Array(*some_list) and inferred types of the result contains <nothing> or an error is reported at the call site.

Does this still happen even with the unbound typevar removed from _CDataMeta.__[r]mul__ (#1986)? Now that those methods return Type[Array[Any]] instead of Type[Array[_CT]], there should be no type inference anymore when creating instances of those types, right?

PS: Is there a way to ask mypy to print out the type that it inferred for a variable/expression in the code? I don't have a lot of experience with mypy yet (I've used typing/Typeshed mostly through PyCharm).

JelleZijlstra commented 6 years ago

For your last question, you can use reveal_type(some_expression). It's a magical function that will make mypy print out the inferred type of some_expression.

ilevkivskyi commented 6 years ago

In that respect, _CArgObject might be a better name than _cparam

I want this name (and all other private names) to be familiar for people who works with ctypes (but I don't so I can't advise here).

Does this still happen even with the unbound typevar removed from _CDataMeta.__[r]mul__ (#1986)? Now that those methods return Type[Array[Any]] instead of Type[Array[_CT]], there should be no type inference anymore when creating instances of those types, right?

These two should be independent. Array is generic, so whenever you write a = Array(1, 2, 3) or lst = [1, 2, 3]; a = Array(*lst) mypy infers that a is Array[int]. I tried to find some simple repro for <nothing> but can't find anything yet. But maybe indeed the offending call was like (some_ctype * 4)(1, 2, 3, 4), I don't have the code that caused this at hand, so let us pospone this until (if) this happens again.

dgelessus commented 6 years ago

whenever you write a = Array(1, 2, 3) or lst = [1, 2, 3]; a = Array(*lst)

That's not actually possible like that, you cannot call ctypes.Array directly:

>>> import ctypes
>>> ctypes.Array(1, 2, 3)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: abstract class

The only supported way to create ctypes arrays is to create a concrete array type first (using ctype * length) and then call that to create the array instance. Since that way the element type is always written out explicitly, there should be no inference needed for the array type parameter. The only issue is that right now the array element type is always set to Any, since I couldn't figure out a way to "share" the type variable properly...

ilevkivskyi commented 6 years ago

The only issue is that right now the array element type is always set to Any, since I couldn't figure out a way to "share" the type variable properly...

You can try annotating cls: Type[_CT] (and keep _CT in the return type). There is a chance that will work, I think at some point we decided to allow this for metaclasses, since this is a very frequent pattern (all classes with a given metaclass inherit from a single "root" class, the latter can be used as an upper bound for type variables). This of course introduces same unsafety, but it is pretty minor, and I think we had some ideas about how to manage it.

ilevkivskyi commented 6 years ago

Hm , actually this still prohibited, probably we have an issue to allow this, anyway I think a # type: ignore is perfectly good here.

dgelessus commented 6 years ago

Yep, sadly that doesn't work. I tried that at first, but mypy complained (for a valid reason):

    # TODO The return type is not accurate. The method definition *should* look like this:
    # def __mul__(cls: Type[_CT], other: int) -> Type[Array[_CT]]: ...
    # but that is not valid, because technically a _CDataMeta might not be a Type[_CT].
    # This can never actually happen, because all _CDataMeta instances are _CData subclasses, but a typechecker doesn't know that.
ilevkivskyi commented 6 years ago

Just to clarify, if you put # type: ignore on the method definition line, it will silence the error

The erased type of self 'Type[tst26.C]' is not a supertype of its class 'tst26.Meta'

But it will work everywhere else correctly.

dgelessus commented 6 years ago

Thanks for the tip, with the ignore comment it's working perfectly.

Except that I have a different problem now - for arrays of "primitive-ish" types (_SimpleCData), the type accepted by the constructor and returned by __getitem__ is not the same as the type parameter. For example, the elements of an Array[c_int] are Python ints, not c_ints. However, for more complex element types (structures, arrays, etc.) this automatic conversion doesn't happen - the elements of an Array[somestruct] are somestructs. I think I have an idea how to solve that though, will have to play around a bit.