DOC: Generic zmq.Context is unclear

[KitB]

This is a pyzmq bug

• This is a pyzmq-specific bug, not an issue of zmq socket behavior. Don't worry if you're not sure! We'll figure it out together.

What pyzmq version?

25.1.2

What libzmq version?

4.3.4

Python version (and how it was installed)

3.10.12 from deadsnakes ppa

OS

Ubuntu 22.04.4

What happened?

In the process of switching an existing zmq codebase over to use zmq.asyncio I would expect a type error to be thrown anywhere that I have tried to provide a zmq.asyncio.Context or zmq.asyncio.Socket where I have annotated something as a zmq.Context or zmq.Socket but because zmq.asyncio.{Context,Socket} subclasses zmq.{Context,Socket} it is considered a valid type.

Code to reproduce bug

from dataclasses import dataclass
import zmq
import zmq.asyncio

@dataclass
class ServerManager:
    context: zmq.Context

ctx = zmq.asyncio.Context()
server_manager = ServerManager(context=ctx)  # Should be a type error but isn't

Traceback, if applicable

No response

More info

No response

[minrk]

Yeah, type-wise zmq.Context and zmq.Socket are generics, so accept any implementation.

I don't know the best way to handle the fact that the base classes are indeed base classes and have a default implementation, but I believe the annotation that only accepts the default implementation is

context: zmq.Context[zmq.Socket[bytes]]

This will accept zmq.Context(), but reject zmq.asyncio.Context().

Liskov substitution makes the false assumption that subclasses are subtypes, which isn't a valid assumption.

[KitB]

I'm not sure of the historicity of this but: isn't the LSP making an assertion about how things should be done rather than making an assumption about how they are done?

[KitB]

I think python/typing#241 makes a point that is a middle ground (and/or is just the point you were getting at) that we use subclassing to provide default implementations of things without intending it to indicate subtyping.

And I've just read to the bottom of that issue and seen that you've made this exact point there.

Could it not be argued that the current implementation has the same problem as the "Has a" implementation in that you've had to provide type overrides for a bunch of methods on zmq._future._AsyncSocket? (I've also noticed that recv_json is missing from those type overrides)

[KitB]

context: zmq.Context[zmq.Socket[bytes]]

The issue with this solution is that my plan was to just throw a zmq.asyncio.Context in and see where the type system complained in order to find the places where I needed to change the annotation. Admittedly this isn't a major problem, I can just do it with a grep but it might be harder if my codebase were larger/more complex.

[minrk]

I think you can catch errors with your previous code if you enable stricter type checking. Using the generic zmq.Context will fail if you check it with --disallow-any-generics:

mypy --disallow-any-generics test.py
test.py:7: error: Missing type parameters for generic type "Context"  [type-arg]

Also because they are generics, the return type of the socket methods will all be Any unless you specify them further, so you don't get much benefit of zmq types if you use the base Generic. Other strict options to disallow Any could catch the lack of typing, too.

For example:

from dataclasses import dataclass
import zmq
import zmq.asyncio

@dataclass
class ServerManager:
    generic_context: zmq.Context
    sync_context: zmq.Context[zmq.Socket[bytes]]
    async_context: zmq.asyncio.Context
    
    def recv(self) -> bytes:
        sock = self.generic_context.socket(zmq.PUSH)
        reveal_type(sock.recv()) # Any
        sock2 = self.sync_context.socket(zmq.PUSH)
        reveal_type(sock2.recv()) # bytes
        sock3 = self.async_context.socket(zmq.PUSH)
        reveal_type(sock3.recv()) # Awaitable[Union[builtins.bytes, zmq.sugar.frame.Frame]

[KitB]

I see what you mean. I got distracted by talking about LSP and failed to take on board that you said they were generics.

[minrk]

Do you think there's anything left to do here? zmq.asyncio.Context is a subtype of the Generic zmq.Context, and the resulting socket method returns Awaitable[bytes | Frame] is a sub-type of the Generic's Any, so type substitution is satisfied. What's weird and reasonable to be surprised/confused by is that zmq.Context.__init__ actually returns the resolved zmq.Context[zmq.Socket[bytes]] and not actually the obvious type zmq.Context. But Python typing is just not very good at representing this kind of thing. I'm not sure what can be done better, but I'm always open to ideas.

[KitB]

It might be useful to have a little more documentation on the type annotations we should use, though I may just have missed it.

Potentially I think it may be more immediately obvious how to use the types if you had, say, a base class and then a synchronous subclass and an asynchronous subclass so they wouldn't typecheck as each other. (specifically I think this may be advantageous because in my experience python devs aren't used to using generics outside of collections -- though with recent improvements to the ability to express generics this may change).

I certainly wouldn't complain if you just closed the issue as wontfix; there is a functional solution to the problem, it's just not what I expected.

[minrk]

Docs are a good point. I'm not particularly inclined to change code to suit typing tools when there's no actual problem with the code itself, just in the typing tools ability to describe it. I can't think of a way that wouldn't break things, and not breaking things is the single most important thing in pyzmq right now.

[minrk]

#1970 adds a note in the docs and SyncContext / SyncSocket TypeAliases for convenience