Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for partial/full specialization of Generic interfaces #97

Closed
wants to merge 1 commit into from
Closed

Add support for partial/full specialization of Generic interfaces #97

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
2.2.0 (UNRELEASED)
------------------

* Added support for interfaces that subclass ``Generic`` and partial/full implementations (meaning the implementation itself is not ``Generic``). Only for Python 3.7+.
* Added support for Python 3.10.

2.1.0 (2021-03-19)
Expand Down
11 changes: 11 additions & 0 deletions src/oop_ext/interface/_interface.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -684,6 +684,17 @@ def __GetInterfaceMethodsAndAttrs(
if attr in self.INTERFACE_OWN_METHODS:
continue

# error: Argument 2 to "issubclass" has incompatible type "<typing special form>"; expected "_ClassInfo" [arg-type]
# This fails during type-checking, but is valid at runtime; let's keep this
# check to ensure we do not break existing cases by accident.
py37_plus = sys.version_info[:2] >= (3, 7)
if py37_plus and issubclass(interface, Generic): # type:ignore[arg-type]
# Do not check some specific methods that Generic declares,
# in case we have a Generic interface and a partial/complete
# specialization as implementation (see testGenericSupport).
if attr in ("__class_getitem__", "__init_subclass__"):
continue

val = getattr(interface, attr)

if type(val) in self._ATTRIBUTE_CLASSES:
Expand Down
32 changes: 32 additions & 0 deletions src/oop_ext/interface/_tests/test_interface.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import re
import sys
import textwrap
from typing import Any
from typing import List

import pytest
Expand Down Expand Up @@ -1077,3 +1079,33 @@ def AfterCaption(*args):

assert Foo.GetCaption() == "Foo"
assert Foo().GetValues("m") == [0.1, 10.0]


@pytest.mark.skipif(
sys.version_info[:2] <= (3, 6), reason="Only supported in Python 3.7+"
)
def testGenericSupport() -> None:
from typing import Generic, TypeVar

T = TypeVar("T")

class AnInterface(Generic[T], interface.Interface, interface.TypeCheckingSupport):
def foo(self, v: T) -> T:
...

@interface.ImplementsInterface(AnInterface)
class Specialization:
Copy link
Member Author

@nicoddemus nicoddemus May 15, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure this is 100% correct: the interface declares that every implementation should support any type as the parameter to foo, like FooGeneric does below.

Specialization is specializing T to str, which violates the AnInterface interface.

FooGeneric works out of the box now, without the changes in _interface.py in this PR.

I'm not sure the use case for Specialization is totally valid.

Copy link

@mv-per mv-per May 15, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does work, but let's say I wrongfully implement Specialization as:

    @interface.ImplementsInterface(AnInterface)
    class Specialization:
        def foo(self, v: int) -> str:
            return str(v) + "bar"

The above code works on runtime and on type_checking. However, it's clear that foo(self, v: int) -> str differs from foo(self, v: T) -> T but neither mypy or interface can see this Liskov substitution principle violation.

If we use Protocol directly, we would have something like:

    from typing import TypeVar, Protocol

    T = TypeVar("T")

    class AnInterface(Protocol[T]):
        def foo(self, v: T) -> T:
            ...

    class Specialization(AnInterface[str]):
        def foo(self, v: int) -> str:
            return str(v) + "bar"

    spec = Specialization()
    assert spec.foo(1) == "1bar"

what would cause:

src/oop_ext/interface/_tests/test_interface.py:1129: error: Argument 1 of "foo" is incompatible with supertype "AnInterface"; supertype defines the argument type as "str"  [override]
src/oop_ext/interface/_tests/test_interface.py:1129: note: This violates the Liskov substitution principle
src/oop_ext/interface/_tests/test_interface.py:1129: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#incompatible-overrides

Seems like mypy is not properly checking the interface's structure

Copy link
Member Author

@nicoddemus nicoddemus May 15, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but neither mypy or interface can see this Liskov substitution principle violation.

mypy does not know about our Interface implementation at all, that's why -- it cannot konw that Specialization and AnInterface are related at all.

For new interface hierarchies, we should consider using plain ABC classes and/or Protocols, as they are natively interpreted by mypy.

(Btw in you example you are not supposed to subclass from AnInterface).

def foo(self, v: str) -> str:
return v + "bar"

@interface.ImplementsInterface(AnInterface)
class FooGeneric(Generic[T]):
def foo(self, v: T) -> T:
return v

spec = Specialization()
assert spec.foo("hello") == "hellobar"

generic = FooGeneric[Any]()
assert generic.foo("hello") == "hello"
assert generic.foo(10) == 10