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

Native python types #8559

Closed
wants to merge 15 commits into from
Closed

Native python types #8559

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
2a8177c
Native python types
wouterdb Jan 6, 2025
708d8f2
Added tests
wouterdb Jan 6, 2025
a5a4f6e
Cleanup
wouterdb Jan 6, 2025
eeb132d
Fixes
wouterdb Jan 6, 2025
61f0984
Update src/inmanta/plugins.py
wouterdb Jan 6, 2025
6d6efc1
Fix pep8
wouterdb Jan 6, 2025
2daf160
Merge branch 'issue/native_types' of github.com:inmanta/inmanta-core …
wouterdb Jan 6, 2025
8bd58d3
Fix eq and hash
wouterdb Jan 7, 2025
cc4d87a
Fixes
wouterdb Jan 7, 2025
bfd4e55
fixes
wouterdb Jan 7, 2025
94ac77f
Update src/inmanta/ast/type.py
wouterdb Jan 7, 2025
6d61bbd
more tests
wouterdb Jan 7, 2025
6f71a84
Merge branch 'issue/native_types' of github.com:inmanta/inmanta-core …
wouterdb Jan 7, 2025
d430ed2
Restrict list types
wouterdb Jan 7, 2025
2aa2eb9
change imports
wouterdb Jan 7, 2025
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
7 changes: 1 addition & 6 deletions mypy-baseline.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,15 +85,11 @@ src/inmanta/ast/__init__.py:0: error: Function is missing a return type annotati
src/inmanta/ast/type.py:0: error: List item 0 has incompatible type "type[float]"; expected "Callable[[object], Number]" [list-item]
src/inmanta/ast/type.py:0: error: List item 0 has incompatible type "type[float]"; expected "Callable[[object], object]" [list-item]
src/inmanta/ast/type.py:0: error: Incompatible types in assignment (expression has type "Sequence[Callable[[object | None], object]]", base class "Number" defined the type as "Sequence[Callable[[object], Number]]") [assignment]
src/inmanta/ast/type.py:0: error: Function is missing a return type annotation [no-untyped-def]
src/inmanta/ast/type.py:0: note: Use "-> None" if function does not return a value
src/inmanta/ast/type.py:0: error: Call to untyped function "__init__" in typed context [no-untyped-call]
src/inmanta/ast/type.py:0: error: Return type "str | None" of "type_string" incompatible with return type "str" in supertype "List" [override]
src/inmanta/ast/type.py:0: error: Function is missing a return type annotation [no-untyped-def]
src/inmanta/ast/type.py:0: error: Function is missing a type annotation [no-untyped-def]
src/inmanta/ast/type.py:0: error: Item "Locatable" of "Locatable | None" has no attribute "name" [union-attr]
src/inmanta/ast/type.py:0: error: Item "None" of "Locatable | None" has no attribute "name" [union-attr]
src/inmanta/ast/type.py:0: error: Call to untyped function "List" in typed context [no-untyped-call]
src/inmanta/ast/type.py:0: error: "List" expects no type arguments, but 1 given [type-arg]
src/inmanta/ast/type.py:0: error: Incompatible types in assignment (expression has type "list[Never]", variable has type "inmanta.ast.type.List") [assignment]
src/inmanta/ast/type.py:0: error: "inmanta.ast.type.List" has no attribute "append" [attr-defined]
Expand Down Expand Up @@ -788,12 +784,11 @@ src/inmanta/protocol/rest/client.py:0: error: Argument "result" to "Result" has
src/inmanta/plugins.py:0: error: Missing type parameters for generic type "ResultVariable" [type-arg]
src/inmanta/plugins.py:0: error: Argument 1 to "add_function" of "PluginMeta" has incompatible type "PluginMeta"; expected "type[Plugin]" [arg-type]
src/inmanta/plugins.py:0: error: "type[Plugin]" has no attribute "__fq_plugin_name__" [attr-defined]
src/inmanta/plugins.py:0: error: Call to untyped function "List" in typed context [no-untyped-call]
src/inmanta/plugins.py:0: error: Non-overlapping identity check (left operand type: "type[object]", right operand type: "<typing special form>") [comparison-overlap]
src/inmanta/plugins.py:0: error: Argument 1 to "issubclass" has incompatible type "Any | None"; expected "type" [arg-type]
src/inmanta/plugins.py:0: error: Argument 1 to "issubclass" has incompatible type "Any | None"; expected "type" [arg-type]
src/inmanta/plugins.py:0: error: Incompatible types in assignment (expression has type "tuple[Any, ...]", variable has type "list[type[object]]") [assignment]
src/inmanta/plugins.py:0: error: Call to untyped function "List" in typed context [no-untyped-call]
src/inmanta/plugins.py:0: error: Argument 1 to "issubclass" has incompatible type "Any | None"; expected "type" [arg-type]
src/inmanta/plugins.py:0: error: Invalid index type "object" for "dict[str | None, Type]"; expected type "str | None" [index]
src/inmanta/plugins.py:0: error: Argument 1 to "to_dsl_type" has incompatible type "object"; expected "type[object]" [arg-type]
src/inmanta/plugins.py:0: error: Incompatible return value type (got "Type | None", expected "Type") [return-value]
Expand Down
4 changes: 2 additions & 2 deletions src/inmanta/ast/type.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ def type_string(self) -> str:
def __eq__(self, other: object) -> bool:
if not isinstance(other, NamedType):
return False
return self.get_full_name() == other
return self.get_full_name() == other.get_full_name()

def __hash__(self) -> int:
return hash(self.get_full_name())
Expand Down Expand Up @@ -398,7 +398,7 @@ class List(Type):
This class refers to the list type used in plugin annotations. For the list type in the Inmanta DSL, see `LiteralList`.
"""

def __init__(self):
def __init__(self) -> None:
Type.__init__(self)

def validate(self, value: Optional[object]) -> bool:
Expand Down
38 changes: 24 additions & 14 deletions src/inmanta/plugins.py
View file
Open in desktop
sanderr marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,11 @@ def __eq__(self, other: object) -> bool:
int: inmanta_type.Integer(),
bool: inmanta_type.Bool(),
dict: inmanta_type.TypedDict(inmanta_type.Type()),
Mapping: inmanta_type.TypedDict(inmanta_type.Type()),
collections.abc.Mapping: inmanta_type.TypedDict(inmanta_type.Type()),
Copy link
Contributor

Choose a reason for hiding this comment

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

Something weird is going on here. Mapping should be from collections.abc. I think it's good to support both, but then the qualification must be the other way around (or both qualified if you prefer). Because it's very likely that the import will be updated at some point.

Copy link
Contributor

Choose a reason for hiding this comment

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

Same for sequence.

list: inmanta_type.List(),
Sequence: inmanta_type.List(),
collections.abc.Sequence: inmanta_type.List(),
object: inmanta_type.Type(),
}

Expand Down Expand Up @@ -277,26 +281,32 @@ def to_dsl_type(python_type: type[object]) -> inmanta_type.Type:

# dict
if issubclass(origin, collections.abc.Mapping):
args = typing_inspect.get_args(python_type)
if not args:
return inmanta_type.TypedDict(inmanta_type.Type())
if origin in [collections.abc.Mapping, dict, typing.Mapping]:
args = typing_inspect.get_args(python_type)
if not args:
return inmanta_type.TypedDict(inmanta_type.Type())

if not issubclass(args[0], str):
raise TypingException(
None, f"invalid type {python_type}, the keys of any dict should be 'str', got {args[0]} instead"
)
if not issubclass(args[0], str):
raise TypingException(
None, f"invalid type {python_type}, the keys of any dict should be 'str', got {args[0]} instead"
)

if len(args) == 1:
return inmanta_type.TypedDict(inmanta_type.Type())
if len(args) == 1:
return inmanta_type.TypedDict(inmanta_type.Type())

return inmanta_type.TypedDict(to_dsl_type(args[1]))
return inmanta_type.TypedDict(to_dsl_type(args[1]))
else:
raise TypingException(None, f"invalid type {python_type}, dictionary types should be Mapping or dict")
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice. I hadn't considered this, but it's good to have a clear message!

One suggestion

Suggested change
raise TypingException(None, f"invalid type {python_type}, dictionary types should be Mapping or dict")
raise TypingException(None, f"invalid type {python_type}, dictionary types should be Mapping (preferred) or dict")

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 think the order implies preference?

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess. I mostly meant to say that I think we should discourage the dict, even if we accept it, because it's simply not accurate.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could even not mention the dict, but it is the most familiar to most people


# List
if issubclass(origin, collections.abc.Sequence):
args = typing.get_args(python_type)
if not args:
return inmanta_type.List()
return inmanta_type.TypedList(to_dsl_type(args[0]))
if origin in [collections.abc.Sequence, list, typing.Sequence]:
Copy link
Contributor

Choose a reason for hiding this comment

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

Not Collection?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Collection also contains Mapping and Set, so I think that is a bit too wide?

Copy link
Contributor

Choose a reason for hiding this comment

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

But we're not accepting subclasses anymore, only those exact annotations. A list is a collection, so that would be acceptable for input args. And we can easily construct lists from it so it's acceptable as return type.

But I'm not too attached to it, it's not that common.

args = typing.get_args(python_type)
if not args:
return inmanta_type.List()
return inmanta_type.TypedList(to_dsl_type(args[0]))
else:
raise TypingException(None, f"invalid type {python_type}, list types should be Sequence or list")

# Set
if issubclass(origin, collections.abc.Set):
Expand Down
16 changes: 16 additions & 0 deletions tests/compiler/test_plugin_types.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,14 @@ def test_conversion():
assert inmanta_type.List() == to_dsl_type(list)
assert inmanta_type.TypedList(inmanta_type.String()) == to_dsl_type(list[str])
assert inmanta_type.TypedList(inmanta_type.String()) == to_dsl_type(Sequence[str])
assert inmanta_type.List() == to_dsl_type(Sequence)
assert inmanta_type.List() == to_dsl_type(collections.abc.Sequence)
assert inmanta_type.TypedList(inmanta_type.String()) == to_dsl_type(collections.abc.Sequence[str])
assert inmanta_type.TypedDict(inmanta_type.Type()) == to_dsl_type(dict)
assert inmanta_type.TypedDict(inmanta_type.Type()) == to_dsl_type(Mapping)
assert inmanta_type.TypedDict(inmanta_type.String()) == to_dsl_type(dict[str, str])
assert inmanta_type.TypedDict(inmanta_type.String()) == to_dsl_type(Mapping[str, str])

assert inmanta_type.TypedDict(inmanta_type.String()) == to_dsl_type(collections.abc.Mapping[str, str])

assert Null() == to_dsl_type(Union[None])
Expand All @@ -48,3 +52,15 @@ def test_conversion():

with pytest.raises(RuntimeException):
to_dsl_type(set[str])

class CustomList[T](list[T]):
pass

class CustomDict[K, V](Mapping[K, V]):
pass

with pytest.raises(RuntimeException):
to_dsl_type(CustomList[str])

with pytest.raises(RuntimeException):
to_dsl_type(CustomDict[str, str])