Change from ABC to protocol.

[BSchilperoort]

Python nowadays supports "protocols". Protocols are mostly a typing thing, and allow you to specify that a class follows the protocol without explicitly being a subclass.

When using the BMI as an abstract base class, you currently need to use the following syntax:

def update_bmi(bmi: BmiABC) -> None:
    bmi.update()

class BmiImplementation(BmiABC):
    def update(self) -> None:
        ....

update_bmi(BmiImplementation())  # happy type checker

If you don't subclass BMI, type checkers will get angry at this, as BmiImplementation is not a subclass of BmiABC:

def update_bmi(bmi: BmiABC) -> None:
    bmi.update()

class BmiImplementation:
    def update(self) -> None:
        ....

update_bmi(BmiImplementation())  # sad type checker

However, if you specify that Bmi is a subclass of typing.protocol, the following syntax passes type checkers:

def update_bmi(bmi: BmiProtocol) -> None:
    bmi.update()

class BmiImplementation:
    def update(self) -> None:
        ...

update_bmi(BmiImplementation())  # happy type checker

This is because type checkers will check if BmiImplementation follows the BmiProtocol: does it have all methods that are in the protocol, and do all the methods have the correct typing.

To implement this, all that we would need to change is;

from typing import Protocol

class Bmi(Protocol):

Note that you can still subclass from Protocol like you could from an ABC.

[mcflugen]

@BSchilperoort I think this is good idea. I have a branch that does this and will create a pull request for you to have a look at (but probably not until next week).

[aaraney]

While I share a general grumble towards ABC classes and generally use Protocols in my day to day work, i'm not sold this really improves things (i'm probably missing something 😅). This would be a breaking change that I think may hurt more than its helps.

For example, there is nothing stopping someone today from writing a "thin" Protocol at the consumer end to limit the scope of methods required from a bmipy.Bmi. So, building on the aforementioned example, if a function just needs access to the update() method on some instance of a bmipy.Bmi subclass, at the consumer end why not just write:

import typing

class BmiUpdater(typing.Protocol):
  def update(self) -> None: ...

def update_bmi(bmi: BmiUpdater) -> None:
    bmi.update()

update_bmi(SomeBmiSubclass()) # no type error

Sure, there is more typing involved (no pun intended :)) but doesn't this accomplish the same goal?

As for breaking changes, one drawback / feature (depends how you look at it) of Protocols is an incomplete implementation of a Protocol does not throw a runtime error. An incomplete implementation of an abc.ABC throws a TypeError. So, you are effectively changing the expected runtime behavior that some software that uses BMI is relying on. So, you turn turn what once was a runtime error into a type checking exercise. For example:

ABC example

import abc
import inspect

class Base(abc.ABC):
    @abc.abstractmethod
    def foo(self) -> str: ...

    @abc.abstractmethod
    def bar(self) -> str: ...

class Subclass(Base):
    def foo(self):
        return "horray"

assert inspect.isabstract(Subclass) # passes

Subclass() # raises TypeError: Can't instantiate abstract class Subclass with abstract method bar

Protocol example

import typing
import inspect

class Base(typing.Protocol):
    def foo(self) -> str: ...

    def bar(self) -> str: ...

class Subclass(Base):
    def foo(self):
        return "horray"

assert inspect.isabstract(Subclass) # raises

Subclass() # typing error, but no runtime error

This feels like if we could go back and re-write BMI from scratch, perhaps you would use a Protocol instead of an ABC. But, it seems the trouble of switching is likely not worth the pain. Thoughts?

[BSchilperoort]

Protocol example

I think you missed one thing, @aaraney. When subclassing Protocol, you can still make use of abstractmethod:

import typing
import inspect
from abc import abstractmethod

class Base(typing.Protocol):
    @abstractmethod
    def foo(self) -> str: ...

    @abstractmethod
    def bar(self) -> str: ...

class Subclass(Base):
    def foo(self):
        return "horray"

assert inspect.isabstract(Subclass)  # passes

Subclass()  # raises TypeError: Can't instantiate abstract class Subclass with abstract method bar

I would still recommend most users to subclass the abstract BMI, especially more novice users, but by making it a protocol we can validate a BMI implementation without strictly needing to subclass it (using a type checker).

---

This would be a breaking change that I think may hurt more than its helps.

As far as I can see, the only way this breaks things is that issubclass(Subclass, abc.ABC) now returns False. Please correct me if I'm wrong though.

[aaraney]

When subclassing Protocol, you can still make use of abstractmethod:

Yeah, that's fair. I'd not considered decorating a protocol like this. Nice catch and interesting thought!

As far as I can see, the only way this breaks things is that issubclass(Subclass, abc.ABC) now returns False. Please correct me if I'm wrong though.

Great catch! Additionally Base needs to be decorated with typing.runtime_checkable to maintain the issubclass(Subclass, Base) relationship when implicitly implementing Base.

import typing
from abc import abstractmethod

# need this
@typing.runtime_checkable
class Base(typing.Protocol):
    @abstractmethod
    def foo(self) -> str: ...

    @abstractmethod
    def bar(self) -> str: ...

class Subclass:
    def foo(self):
        return "hooray"

    def bar(self) -> str:
        return "bar"

assert issubclass(Subclass, Base) # without `@typing.runtime_checkable` this fails

The docs point out there is a performance degradation when using typing.runtime_checkable. I don't think this really matters, I mention it only to add to the list of the pros and cons.

Note: An isinstance() check against a runtime-checkable protocol can be surprisingly slow compared to an isinstance() check against a non-protocol class. Consider using alternative idioms such as hasattr() calls for structural checks in performance-sensitive code.

---

Aside, if I saw a Protocol decorated with runtime_checkable and abstractmethods in the wild I would certainly scratch my head. Not to get on the subject of what is and is not "pythonic" but at face value this feels like an anti-pattern.

If the goal is to remove the need for implementations to rely on bmipy as a dependency I think this could make sense. Any changes to bmipy.Bmi and versioning in general seem like they would be a headache though.

I don't want to lose the plot, @BSchilperoort! I use BMI in a very constrained way and have my own biases.

• Is one the goals to remove the need to depend on bmipy?
• Can you speak to some use cases you are thinking of?

[BSchilperoort]

Great catch! Additionally Base needs to be decorated with typing.runtime_checkable to maintain the issubclass(Subclass, Base) relationship when implicitly implementing Base.

if I saw a Protocol decorated with runtime_checkable and abstractmethods in the wild I would certainly scratch my head.

I did not realize that this would break when using a protocol... That does make me doubt if this would be a good idea now.

---

Is one the goals to remove the need to depend on
Can you speak to some use cases you are thinking of?

It would indeed remove runtime dependence on bmipy, but I guess the workaround for this would be;

from typing import TYPE_CHECKING

if TYPE_CHECKING:
    from bmipy import Bmi

def func(bmi: "Bmi"):
    bmi.initialize("config.toml")

Can you speak to some use cases you are thinking of?

In case BMI was a protocol, it would be easier to implement a BMI "partially", with less boilerplate required (#61). A BMI that exposes a rectilinear dataset for example, does not have to implement any setter functions or node/edge functionality (~10 methods). Protocols can cover part of the whole implementation, and implementations can be checked just by if they comply to the required protocol.