Type annotations: Use the common parent or a type union for related classes? Pros and cons

Python Help
1

I’m currently annotating legacy code with the help of pyre infer.

I encountered a set of hierarchically organized classes:

Descriptor
+ -- EmptyDescriptor
+ -- IntDescriptor
+ -- + -- FloatDescriptor
+ -- + -- EnumDescriptor
+ -- StringDescriptor
+ -- ErrorDescriptor
+ ...

There are many such classes. Now, there is a simple function:

def get(param_name):
    return _cache[param_name]

Pyre suggests the following annotation:

def get(param_name: str) -> (
    EmptyDescriptor
    | IntDescriptor
    | FloatDescriptor
    | EnumDescriptor
    | StringDescriptor
    | ErrorDescriptor
    | ... the rest of them ...
): ...

Two ways to simplify this annotation come to mind:

1.) Use the common parent:

def get(param_name: str) -> Descriptor

2.) Create a type union:

GeneralDescriptor: TypeAlias = EmptyDescriptor | IntDescriptor | FloatDescriptor | ...

Are these two approaches equivalent, or are there nuances I might be overlooking?

2

They are equivalent unless Descriptor contains a Literal type identifier. I.e. something like

class EmptyDescriptor(Descriptor):
    type: ClassVar[Literal["empty"]]
    ...

In that case you would end up with a tagged union and can narrow from the union to a specific type that’s part of the union using desc.type == "empty" instead of isinstance(desc, EmptyDescriptor).

So in that sense a tagged union is more powerful, but it also means any additional subclasses won’t end up in the tagged union unless you add them later on, so if the classes are intended to be a complete set, that’s not user extensible, a tagged union can make sense, otherwise you might as well use the base type and potentially extend it with any abstract attributes/methods that are provided by all its subclasses, so you can rely on that attribute/method being present without having to type narrow to one of the subclasses.

3

Thank you for your reply!

4

Even without a Literal attribute, the union is also useful if users have like an if-else chain doing isinstance checks. In that case the checker can narrow down the union in each branch, allowing you to use assert_never at the end to verify all cases were handled. That doesn’t work with the base class, because the type checker knows that it is potentially possible other code may have also inherited from the base class, so it can’t narrow anything.