Skip to content

A few tweaks to the MLIR .pyi files #110488

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

Merged
merged 3 commits into from
Oct 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 1 addition & 2 deletions mlir/python/mlir/_mlir_libs/_mlir/__init__.pyi
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
from typing import List

globals: "_Globals"

class _Globals:
dialect_search_modules: List[str]
dialect_search_modules: list[str]
def _register_dialect_impl(self, dialect_namespace: str, dialect_class: type) -> None: ...
def _register_operation_impl(self, operation_name: str, operation_class: type) -> None: ...
def append_dialect_search_prefix(self, module_name: str) -> None: ...
Expand Down
9 changes: 4 additions & 5 deletions mlir/python/mlir/_mlir_libs/_mlir/dialects/pdl.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
# See https://llvm.org/LICENSE.txt for license information.
# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception

from typing import Optional

from mlir.ir import Type, Context

Expand All @@ -26,15 +25,15 @@ class AttributeType(Type):
def isinstance(type: Type) -> bool: ...

@staticmethod
def get(context: Optional[Context] = None) -> AttributeType: ...
def get(context: Context | None = None) -> AttributeType: ...
Copy link
Contributor

Choose a reason for hiding this comment

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

this syntax requires 3.10? 3.11? ie would break 3.8, 3.9.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It will break if the type checker enforces a specific Python version on type stubs. Not sure if that happens in practice.

Note also that this syntax is already used in ir.pyi.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there a reason why MLIR wants to target 3.8+ for its Python bindings? 3.8 is EOL in a months and 3.9 is only receiving security updates before EOLing next year.

Copy link
Contributor

Choose a reason for hiding this comment

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

Traditionally, llvm debates such things. I can't remember the specifics of the last time, but generally we've been tracking the eol schedule. Dropping 3.8 on those grounds SGTM.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would also support type stubs only being supported for active versions so long as it doesn't impede use of the library on older versions. I think it is completely reasonable for development efficiency features to not target discontinued versions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Type stubs are not used at all at runtime, so as long as type checkers are happy with the new syntax, it should be fine to use it, right?

Copy link
Contributor

Choose a reason for hiding this comment

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

It will break if the type checker enforces a specific Python version on type stubs.

That's not correct - it will raise a TypeError in just vanilla python:

$ conda create -n throwaway python=3.8
$ conda activate throwaway
(throwaway) $ python -c "def fun(a: int | float): pass"
Traceback (most recent call last):
  File "<string>", line 1, in <module>
TypeError: unsupported operand type(s) for |: 'type' and 'type'

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't have a strong opinion on moving the support window forward - I'm just verifying.

Copy link
Contributor

@makslevental makslevental Oct 1, 2024

Choose a reason for hiding this comment

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

Ah sorry I didn't realize this was in a pyi file. In which case I think you're correct.



class OperationType(Type):
@staticmethod
def isinstance(type: Type) -> bool: ...

@staticmethod
def get(context: Optional[Context] = None) -> OperationType: ...
def get(context: Context | None = None) -> OperationType: ...


class RangeType(Type):
Expand All @@ -53,12 +52,12 @@ class TypeType(Type):
def isinstance(type: Type) -> bool: ...

@staticmethod
def get(context: Optional[Context] = None) -> TypeType: ...
def get(context: Context | None = None) -> TypeType: ...


class ValueType(Type):
@staticmethod
def isinstance(type: Type) -> bool: ...

@staticmethod
def get(context: Optional[Context] = None) -> ValueType: ...
def get(context: Context | None = None) -> ValueType: ...
7 changes: 3 additions & 4 deletions mlir/python/mlir/_mlir_libs/_mlir/dialects/quant.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
# See https://llvm.org/LICENSE.txt for license information.
# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception

from typing import List

from mlir.ir import Type

Expand Down Expand Up @@ -94,15 +93,15 @@ class UniformQuantizedPerAxisType(QuantizedType):

@classmethod
def get(cls, flags: int, storage_type: Type, expressed_type: Type,
scales: List[float], zero_points: List[int], quantized_dimension: int,
scales: list[float], zero_points: list[int], quantized_dimension: int,
storage_type_min: int, storage_type_max: int):
...

@property
def scales(self) -> List[float]: ...
def scales(self) -> list[float]: ...

@property
def zero_points(self) -> List[float]: ...
def zero_points(self) -> list[float]: ...

@property
def quantized_dimension(self) -> int: ...
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
# See https://llvm.org/LICENSE.txt for license information.
# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception

from typing import Optional

from mlir.ir import Type, Context

Expand All @@ -12,15 +11,15 @@ class AnyOpType(Type):
def isinstance(type: Type) -> bool: ...

@staticmethod
def get(context: Optional[Context] = None) -> AnyOpType: ...
def get(context: Context | None = None) -> AnyOpType: ...


class OperationType(Type):
@staticmethod
def isinstance(type: Type) -> bool: ...

@staticmethod
def get(operation_name: str, context: Optional[Context] = None) -> OperationType: ...
def get(operation_name: str, context: Context | None = None) -> OperationType: ...

@property
def operation_name(self) -> str: ...
Loading
Loading