Common checks and union-attr error

[jamesmyatt]

Bug Report / Discussion

I want to write the code shown below where the types of the attributes are checked in a centralised consistent way to avoid duplication. However, this triggers the union-attr error because the type inference within the checking method doesn't propagate to the outer method.

I'm pretty sure that mypy can't (or won't) do anything about this, but I'm not clear about what the proper resolution for this is that avoids (too much) code duplication or lots of extra code (see below for potential workarounds). Maybe something needs to be documented (e.g. via this issue) and possibly added to the common issues.

To Reproduce

class X:
    def __init__(self, x: str | None = None) -> None:
        self.x = x

    def ready(self) -> bool:
        return isinstance(self.x, str) and bool(self.x)

    def show(self) -> None:
        if not self.ready():
            raise RuntimeError("Object is not configured")

        print(repr(self.x.split()))  # <-- union-attr error

This is a simple example where the duplication would be limited, but it's fairly easy to get into situations where the ready method is far more complicated, e.g. might need to check several fields or combinations of fields, and where it's reused many times. Note that the ready method is useful in its own right.

Expected Behavior

Should be ok, in an ideal world.

Actual Behavior

x.py:12: error: Item "None" of "str | None" has no attribute "split"  [union-attr]
Found 1 error in 1 file (checked 1 source file)

Potential Workarounds

Workarounds are to write one of the following:

    def show(self) -> None:
        if not self.ready():
            raise RuntimeError("Object is not configured")

        print(repr(self.x.split())  # type: ignore[union-attr]  # might ignore other potential errors

or

    def show(self) -> None:
        if self.x is None or not self.ready():  # Duplicates logic in ready method
            raise RuntimeError("Object is not configured")

        print(repr(self.x.split()))

or (as suggested in (https://github.com/python/mypy/issues/4805#issuecomment-1018892112))

    def show(self) -> None:
        if not self.ready():
            raise RuntimeError("Object is not configured")
        assert isinstance(self.x, str)  # Duplicates logic in ready method
        # although it's nice that this assert is skipped when using `python -O`

        print(repr(self.x.split())

or (as I think is suggested in https://github.com/python/mypy/issues/15207#issuecomment-1537519614)

    @property
    def x_checked(self) -> str:
        # Do I have to write a property for every attribute?
        if isinstance(self.x, str) and bool(self.x):
            return self.x
        else:
            raise RuntimeError("Object is not configured")

    def ready(self) -> bool:
        try:  # Clumsy
            self.x_checked
        except Exception:
            return False
        else:
            return True

    def show(self) -> None:
        print(repr(self.x_checked.split()))

Note that the following is not a workaround, since it's an implicit Optional.

    def __init__(self, x: str = None) -> None:
        self.x = x

Your Environment

• Mypy version used: 1.5.0 (compiled: yes)
• Mypy command-line flags:
• Mypy configuration options from mypy.ini (and other config files):
• Python version used: 3.10.12

[erictraut]

If you have just one or two pieces of state that are initialized at the time that the object is "ready", you can move the logic to individual property accessors. To avoid duplication, you can move the validation logic to the property implementation rather than repeating it each time you access the property.

class X:
    def __init__(self, x: str | None = None) -> None:
        self._x = x

    @property
    def x(self) -> str:
        if not self._x:
            raise RuntimeError("Object is not configured")
        return self._x

    def show(self) -> None:
        print(repr(self.x.split()))

If your class is managing many pieces of state that are all initially None and then set at the same time that the object becomes "ready", you may prefer to bundle these up into a private object that is set at that time.

from dataclasses import dataclass

@dataclass
class _XState:
    x: str
    y: int

class X:
    def __init__(self, x: str | None = None) -> None:
        self._state = _XState(x, 0) if x else None

    @property
    def state(self) -> _XState:
        if not self._state:
            raise RuntimeError("Object is not configured")
        return self._state

    def show(self) -> None:
        print(repr(self.state.x.split()))
        print(self.state.y)

[Gilwyad]

I am also affected by this issue. The workarounds are not good enough. Is it not suggested to use optional values like this?