graphql-relay-py
graphql-relay-py copied to clipboard
Update typing
The release of mypy 1.0 revealed some issues in the typing that I helped add previously:
- The
PageInfoTypeprotocol is missing some@propertydecorators - The typing on the *Constructor protocols is overly restrictive. It requires the return values to be of specific shapes, but actually these constructors should not be this strict. For instance, one use case for the
PageInfoConstructoris creating a class which uses snake_case instead of camelCase for the properties. The implementation ofgraphql-relay-pylibrary is not opinionated about this, so the typing should not be either. - The
connection_from_arrayandconnection_from_array_slicemight not return aConnectionType, due to 2 above. So@overloadvariants are added here to more accurately match the behavior.
I've also added a Traversable protocol, which specifies and object with a cursor property. This is the minimum requirement for the return of EdgeConstructor, since the code will crash without a cursor.
In addition, updating the Github workflows and black formatting here.
I spent some time attempting to make the typing much more accurate using generic parameters on the various protocols, but parameterized protocols don't work properly when applied to arguments at the moment. Its also currently not possible with mypy to set a default value on an argument with a type variable due to this issue, which makes the number of overloads explode. Hopefully these problems will be resolved and the typing on graphql-relay-py can be further improved.
I'm not sure what should be done about the lint failure here. There's a manifest check that's failing. Maybe setup.py should be deleted from the repo?
Thanks @markedwards, will look into this. Yes, the setup.py should go, I already removed it in GraphQL-core and planned to revisit GraphQL-relay after the next release anyway.
It is possible some or all of the *Type protocols are not useful and should be deprecated or removed. I think this decision should perhaps wait until a fully-typed version of the connection functions is possible though. Ideally, the typing on those should enforce correct usage and right now it’s possible to provide conflicting constructors. Also, the Any return types are silly, but we need to wait for mypy to fix bugs. :-/
Also, probably a good idea to remove 3.6 support and use the latest mypy? And also, add 3.11 testing and support?
I didn't want to do these things in this PR because they are a separate consideration and also I don't fully understand the poetry/tox setup.
Sure. This is already changed in GraphQL-core and will of course also change in GraphQL-relay.
In terms of further improving the typing here, I have to say it's not quite clear to me what the forward path is yet. Clearly this bug is blocking, because connection_type, edge_type and page_info_type need to be generic.
I think what might work in the end is constraining edge_type and page_info_type to the Edge and PageInfo named tuple classes, in the case that connection_type is not passed. And in the case that it is passed, enforcing return type consistency between connection_type, edge_type, and page_info_type. So, this will be a bunch of @overload with a permissive implementation (probably will end up as a bunch of Any).
I don't think the other issue I mentioned is relevant actually. I think its highly unlikely passing default values for a parameterized argument will ever be implemented (that issue is 5 years old), and its also not clear that it would resolve the complex relationship between connection_type, edge_type, and page_info_type in this case anyway.
So I guess I'll take another stab at it when the blocker is resolved.