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.

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

feat(python): support inheritance-based interface implementation #3350

Draft
wants to merge 22 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
7a0ee64
feat(python): support inheritance-based interface implementation
RomainMuller Jan 24, 2022
5e61e84
Merge branch 'main' into rmuller/python-interfaces
Jan 24, 2022
e5259ea
linter fix
RomainMuller Jan 24, 2022
bfa7492
fix snapshot
RomainMuller Jan 25, 2022
5a6b6e3
make generated classes extend the interfaces they implement
RomainMuller Jan 25, 2022
742e12e
Update python.md
Jan 25, 2022
31feec8
Try to be dependency-has-protocol-friendly
RomainMuller Jan 26, 2022
715a3f7
linter fix + version fix
RomainMuller Jan 26, 2022
7c7da80
use issubclass instead of trying to match type
RomainMuller Jan 26, 2022
ae365fc
fix typo
RomainMuller Jan 26, 2022
2424392
try to not use issubclass (won't work in python 3.6)
RomainMuller Jan 26, 2022
9e0d3c5
de-duplicate interfaces to avoid MRO issues
RomainMuller Jan 27, 2022
2bae124
linter fix
RomainMuller Jan 27, 2022
9eaa38e
Merge branch 'main' into rmuller/python-interfaces
Jan 27, 2022
2eb82ed
remove type: ignore[misc]
RomainMuller Jan 28, 2022
261f204
add imssing required imports
RomainMuller Jan 28, 2022
e96331c
fix duplicate type variable issue
RomainMuller Jan 28, 2022
aa4af36
be friendlier with PyCharm
RomainMuller Jan 28, 2022
dd3b01b
Merge branch 'main' into rmuller/python-interfaces
Jan 28, 2022
3d471d8
remove excess type annotations on python.py (not sure how to do this …
RomainMuller Jan 31, 2022
342b5c4
Emit warning when `@jsii.implements` is used but considered legacy form
RomainMuller Feb 8, 2022
5885996
Merge remote-tracking branch 'origin/main' into rmuller/python-interf…
RomainMuller Feb 8, 2022
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
53 changes: 28 additions & 25 deletions gh-pages/content/user-guides/lib-user/language-specific/python.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,45 +16,48 @@ Traditionally, **Python** developers expect to be able to either *implicitly* im
required members, or *explicitly* implement interfaces by simply adding the interface to their class' or interface's
inheritance chain (and implementing all required members):

!!! bug "Incorrect Use"

```py hl_lines="3"
from jsii_dependency import IJsiiInterface

class MyNewClass(IJsiiInterface):
""" Traditional implementation of an interface in Python.

This will not work with interfaces defined by jsii modules, as this will
likely cause a metaclass conflict that the user cannot solve.
"""

# Member implementations...

...
```

The [jsii type system][jsii-type-system] however does not support *structural typing*, and interfaces must **always** be
*explicitly* implemented. In order to correctly declare implementation of an interface from a *jsii module*, the
following syntax is used:

```py hl_lines="1 4"
import jsii
```py hl_lines="3"
from jsii_dependency import IJsiiInterface

@jsii.implements(IJsiiInterface)
class MyNewClass():
""" A jsii-supported implementation of the `IJsiiInterface` interface
class MyNewClass(IJsiiInterface):
""" Traditional implementation of an interface in Python.

This will correctly register the explicit interface implementation on the
type's metadata, and ensure instances get correctly serialized to and from
the jsii kernel.
This will not work with interfaces defined by jsii modules, as this will
likely cause a metaclass conflict that the user cannot solve.
"""

# Member implementations...

...
```

!!! info Legacy Style
When using libraries generated by `jsii-pacmak` version `1.52.1` and earlier, using the inheritance chain style
above may result in metaclass conflicts when performing multiple inheritance. In such cases, the implementation can
be performed using the `@jsii.implements` decorator instead.

```py hl_lines="1 4"
import jsii
from jsii_dependency import IJsiiInterface

@jsii.implements(IJsiiInterface)
class MyNewClass():
""" A jsii-supported implementation of the `IJsiiInterface` interface

This will correctly register the explicit interface implementation on the
type's metadata, and ensure instances get correctly serialized to and from
the jsii kernel.
"""

# Member implementations...

...
```

[jsii-type-system]: ../../../specification/2-type-system.md

## Property Overrides
Expand Down
5 changes: 3 additions & 2 deletions packages/@jsii/python-runtime/src/jsii/_runtime.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,12 @@

import attr

from typing import cast, Any, Callable, ClassVar, List, Optional, Mapping, Type, TypeVar
from typing import cast, Any, Callable, List, Optional, Mapping, Type, TypeVar

from . import _reference_map
from ._compat import importlib_resources
from ._kernel import Kernel
from .python import _ClassPropertyMeta
from ._kernel.types import ObjRef


# Yea, a global here is kind of gross, however, there's not really a better way of
Expand Down Expand Up @@ -144,6 +143,8 @@ def deco(cls):
def interface(*, jsii_type: str) -> Callable[[T], T]:
def deco(iface):
iface.__jsii_type__ = jsii_type
# This interface "implements itself" - this is a trick to ease up implementation discovery.
iface.__jsii_ifaces__ = [iface]
_reference_map.register_interface(iface)
return iface

Expand Down
34 changes: 34 additions & 0 deletions packages/@jsii/python-runtime/tests/test_compliance.py
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,22 @@ def next(self):
return next_


class SubclassNativeFriendlyRandom_Inheritance(
Number, IFriendly, IRandomNumberGenerator
):
def __init__(self):
super().__init__(908)
self.next_number = 100

def hello(self):
return "SubclassNativeFriendlyRandom"

def next(self):
next_ = self.next_number
self.next_number += 100
return next_


@jsii.implements(IFriendlyRandomGenerator)
class PureNativeFriendlyRandom:
"""
Expand Down Expand Up @@ -753,6 +769,10 @@ def test_testInterfaces():
poly.say_hello(SubclassNativeFriendlyRandom())
== "oh, SubclassNativeFriendlyRandom"
)
assert (
poly.say_hello(SubclassNativeFriendlyRandom_Inheritance())
== "oh, SubclassNativeFriendlyRandom"
)
assert poly.say_hello(PureNativeFriendlyRandom()) == "oh, I am a native!"


Expand All @@ -777,6 +797,20 @@ def test_testNativeObjectsWithInterfaces():
assert generator_bound_to_pure_native.next_times100() == 100_000
assert generator_bound_to_pure_native.next_times100() == 200_000

###
# One more time, but this time implementing the interface via "classic" inheritance.
###
subclassed_native = SubclassNativeFriendlyRandom_Inheritance()
generator_bound_to_p_subclassed_object = NumberGenerator(subclassed_native)

assert generator_bound_to_p_subclassed_object.generator is subclassed_native
generator_bound_to_p_subclassed_object.is_same_generator(subclassed_native)
assert generator_bound_to_p_subclassed_object.next_times100() == 10000

# When we invoke nextTimes100 again, it will use the objref and call into the same
# object.
assert generator_bound_to_p_subclassed_object.next_times100() == 20000


def test_testLiteralInterface():
obj = JSObjectLiteralForInterface()
Expand Down
48 changes: 23 additions & 25 deletions packages/jsii-pacmak/lib/targets/python.ts
Original file line number Diff line number Diff line change
Expand Up @@ -327,11 +327,9 @@ abstract class BasePythonClassType implements PythonType, ISortableType {
public readonly pythonName: string,
public readonly spec: spec.Type,
public readonly fqn: string | undefined,
opts: PythonTypeOpts,
{ bases = [] }: PythonTypeOpts,
public readonly docs: spec.Docs | undefined,
) {
const { bases = [] } = opts;

this.bases = bases;
this.members = [];
}
Expand Down Expand Up @@ -508,10 +506,8 @@ abstract class BaseMethod implements PythonBase {
public emit(
code: CodeMaker,
context: EmitContext,
opts?: BaseMethodEmitOpts,
{ renderAbstract = true, forceEmitBody = false }: BaseMethodEmitOpts = {},
) {
const { renderAbstract = true, forceEmitBody = false } = opts ?? {};

const returnType: string = toTypeName(this.returns).pythonType(context);

// We cannot (currently?) blindly use the names given to us by the JSII for
Expand Down Expand Up @@ -874,9 +870,8 @@ abstract class BaseProperty implements PythonBase {
public emit(
code: CodeMaker,
context: EmitContext,
opts?: BasePropertyEmitOpts,
{ renderAbstract = true, forceEmitBody = false }: BasePropertyEmitOpts = {},
) {
const { renderAbstract = true, forceEmitBody = false } = opts ?? {};
const pythonType = toTypeName(this.type).pythonType(context);

// "# type: ignore[misc]" is needed because mypy cannot check decorated things
Expand Down Expand Up @@ -973,7 +968,10 @@ class Interface extends BasePythonClassType {
if (this.separateMembers) {
code.line();
}
member.emit(code, context, { forceEmitBody: true });
member.emit(code, context, {
forceEmitBody: true,
renderAbstract: false,
});
}
} else {
code.line('pass');
Expand All @@ -998,7 +996,8 @@ class Interface extends BasePythonClassType {
toTypeName(b).pythonType({ ...context, typeAnnotation: false }),
);

params.push('typing_extensions.Protocol');
// Interfaces are kind of like abstract classes. We type them the same way.
params.push('metaclass = jsii.JSIIAbstractClass');

return params;
}
Expand All @@ -1009,12 +1008,16 @@ class Interface extends BasePythonClassType {
}

class InterfaceMethod extends BaseMethod {
// Interface members are always abstract
public readonly abstract = true;
protected readonly implicitParameter: string = 'self';
protected readonly jsiiMethod: string = 'invoke';
protected readonly shouldEmitBody: boolean = false;
}

class InterfaceProperty extends BaseProperty {
// Interface members are always abstract
public readonly abstract = true;
protected readonly decorator: string = 'builtins.property';
protected readonly implicitParameter: string = 'self';
protected readonly jsiiGetMethod: string = 'get';
Expand Down Expand Up @@ -1343,14 +1346,6 @@ class Class extends BasePythonClassType implements ISortableType {
}

public emit(code: CodeMaker, context: EmitContext) {
// First we emit our implments decorator
if (this.interfaces.length > 0) {
const interfaces: string[] = this.interfaces.map((b) =>
toTypeName(b).pythonType({ ...context, typeAnnotation: false }),
);
code.line(`@jsii.implements(${interfaces.join(', ')})`);
}

// Then we do our normal class logic for emitting our members.
super.emit(code, context);

Expand Down Expand Up @@ -1409,15 +1404,18 @@ class Class extends BasePythonClassType implements ISortableType {
}

protected getClassParams(context: EmitContext): string[] {
const params: string[] = this.bases.map((b) =>
toTypeName(b).pythonType({ ...context, typeAnnotation: false }),
);
const metaclass: string = this.abstract ? 'JSIIAbstractClass' : 'JSIIMeta';

params.push(`metaclass=jsii.${metaclass}`);
params.push(`jsii_type="${this.fqn}"`);

return params;
return [
...this.bases.map((b) =>
toTypeName(b).pythonType({ ...context, typeAnnotation: false }),
),
...this.interfaces.map((i) =>
toTypeName(i).pythonType({ ...context, typeAnnotation: false }),
),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I realized this particular part of the change would break if the implemented interface is from a library that was generated with the typing_extensions.Protocol metaclass.

Not putting this change in means the type checkers will not accept instances of classes where implementations of interfaces are expected (because typing_extensions.Protocol is what causes structural typing to apply).

I need to find a strategy to be backwards-compatible here, either by conditionally extending interfaces IIF they are not typing_extensions.Protocol types, or by making type-checkers understand the interface is implemented implicitly in all cases.

`metaclass=jsii.${metaclass}`,
`jsii_type="${this.fqn}"`,
];
}

private get proxyClassName(): string {
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading