I have a Processor class that looks like this:

class Processor:
    def __init__(self, payment_table: List[PaymentTable]):
        self.payment_table = payment_table
    
    def get_payment_table(self, year: int, month: int) -> Optional[PaymentTable]:
        ptable = None
        for table in self.payment_tables:
            if table.is_applicable(year, month):
                ptable = table
        return ptable

    def process_payment(self, contract: Contract, year: int, month: int):
        ptable = self.get_payment_table(year, month)
        payment_value = ptable.hour_value * contract.hours
        # Here logic was simplified just to get to the point...

mypy complains (correctly about ptable.hour_value:

Item "None" of "Optional[PaymentTable]" has no attribute "hour_value"

When searching for payment tables with provided arguments (month and year) it's possible that we do not have a payment table that suffices this values. So, the result of get_payment_table could be None, then I should hint the return type as Optional[PaymentTable].

I know of strategies to deal with the problem of not finding a table (i.e. I can raise an error if we doesn't find any payment table). I'm interested here in how should I type hint it for not getting this mypy error.

In other words: you have a method that can return None and you want to use this to get a object for another function to use. How do you build it? How do you type it? What's the pythonic way of doing this? I accept answers that change my code entirely.

CodePudding user response：

You mention one solution, "I can raise an error if we doesn't find any payment table". If you do this, I believe mypy is smart enough to know that subsequent references to ptable will not be None.

    def process_payment(self, contract: Contract, year: int, month: int):
        ptable = self.get_payment_table(year, month)
        if ptable is None:
            raise Exception(f"Could not find payment table with year={year} and month={month}"
        payment_value = ptable.hour_value * contract.hours
        # Here logic was simplified just to get to the point...

If none of your code can handle a missing payment table, then you could instead raise this exception in get_payment_table itself and change the return type to be non-optional.

If some callsites need a non-None payment table but others are fine with Nones, then you might want two versions of get_payment_table, one that returns optional and one that throws.

    def maybe_get_payment_table(self, year: int, month: int) -> Optional[PaymentTable]:
        ptable = None
        for table in self.payment_tables:
            if table.is_applicable(year, month):
                ptable = table
        return ptable

    def get_payment_table(self, year: int, month: int) -> PaymentTable:
        ptable = self.maybe_get_payment_table(year, month)
        if ptable is None:
            raise Exception(f"Could not find payment table with year={year} and month={month}"
        return ptable

CodePudding user response：

Since it's not possible to process a payment if there is no payment table, I'd suggest that the simplest (and hence "pythonic") way of doing this would be to have get_payment_table raise an exception if there is no payment table, and allow that exception to raise from process_payment:

    def get_payment_table(self, year: int, month: int) -> PaymentTable:
        """Get payment table, raise KeyError if none."""
        for table in self.payment_tables:
            if table.is_applicable(year, month):
                return table
        raise KeyError(f"No payment table for {year}/{month}")

    def process_payment(self, contract: Contract, year: int, month: int) -> None:
        """Process payment.  Raises KeyError if no payment table."""
        ptable = self.get_payment_table(year, month)
        payment_value = ptable.hour_value * contract.hours

I used KeyError here but you might prefer to define your own PaymentTableError class instead.

Since get_payment_table now returns PaymentTable (or raises), mypy won't complain about using ptable.hour_value, since it's not possible to reach that line of code with ptable being anything other than a PaymentTable.

If you want process_payment to fail silently instead of raising, you'd do:

    def process_payment(self, contract: Contract, year: int, month: int) -> None:
        """Process payment.  Returns None if no payment table."""
        try:
            ptable = self.get_payment_table(year, month)
        except KeyError:
            return
        payment_value = ptable.hour_value * contract.hours

In this case we don't raise, but mypy is still happy, and the code is pretty explicit about why we're returning None here.