-
Notifications
You must be signed in to change notification settings - Fork 9
feat(spider-py): Define TDL types; Add support for converting Python types into TDL types. #187
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
Changes from 142 commits
Commits
Show all changes
144 commits
Select commit
Hold shift + click to select a range
d2ac198
Update yscope-dev-utils
sitaowang1998 cc8a097
Use boost install task
sitaowang1998 7f68bdc
Update install task variable names
sitaowang1998 2394649
Set CMP0074 to NEW to find boost
sitaowang1998 6ff3e33
Add uv to install script
sitaowang1998 7ae8a78
Fix cpp-lint root paths
sitaowang1998 56502f9
Fix clang-tidy file pattern
sitaowang1998 6f6becf
Limit build parallelism
sitaowang1998 a74523d
Bug fix
sitaowang1998 2345d22
Merge branch 'dep-concurrency' into yscope-dev-utils
sitaowang1998 77ee494
Bug fix
sitaowang1998 38b86c8
Merge branch 'dep-concurrency' into yscope-dev-utils
sitaowang1998 3698339
Bug fix
sitaowang1998 369e9f1
Rename variables to mirror CLP core
sitaowang1998 90aa5a2
Rename variables to mirror clp core
sitaowang1998 1769c95
Merge branch 'dep-concurrency' of github.com:sitaowang1998/spider int…
sitaowang1998 edaa834
Merge branch 'dep-concurrency' into yscope-dev-utils
sitaowang1998 7faac8f
Revert "Merge branch 'dep-concurrency' of github.com:sitaowang1998/sp…
sitaowang1998 f494a90
Add comment for deps parallelism default value
sitaowang1998 eb01bb2
Merge branch 'dep-concurrency' into yscope-dev-utils
sitaowang1998 ff2fe1c
Update yscope-dev-utils
sitaowang1998 d476e42
Merge branch 'main' into yscope-dev-utils
sitaowang1998 850126d
Merge branch 'yscope-dev-utils' into python_lint
sitaowang1998 65841a0
Add latest python lint config files
sitaowang1998 8564fc2
Update ruff lint tasks
sitaowang1998 a6e7d29
Fix ruff lint
sitaowang1998 573b448
Fix ruff lint
sitaowang1998 6ad72c3
Bug fix
sitaowang1998 ba7c6e5
Fix ruff
sitaowang1998 209acb1
Fix ruff
sitaowang1998 406b514
Reformat files
sitaowang1998 66892f5
Remove .inc from cpp linting
sitaowang1998 80f0a10
Merge branch 'yscope-dev-utils' into python_lint
sitaowang1998 3186a42
Add mypy and merge lint and test requirements.txt
sitaowang1998 0925938
Fix mysql connection type
sitaowang1998 6936a9b
Fix socket name type
sitaowang1998 7407090
Fix return type from db cursor
sitaowang1998 6a95b24
Fix db cursor return type
sitaowang1998 f29dcba
Fix mypy import untyped
sitaowang1998 453f209
Fix mypy and Popen
sitaowang1998 d1011c7
Fix generator type hint
sitaowang1998 a7dc642
Fix mypy
sitaowang1998 a7d92c2
Fix ruff
sitaowang1998 1c6213b
Simply socket return types.
sitaowang1998 0f13d07
Merge branch 'main' into python_lint
sitaowang1998 14d6326
Merge branch 'python_lint' into mypy_lint
sitaowang1998 55a727a
Add tombi lint tasks
sitaowang1998 c98fea1
Rename cpp build tasks
sitaowang1998 ef64454
Merge branch 'main' into mypy_lint
sitaowang1998 09ba8ad
Merge branch 'main' into tombi
sitaowang1998 4e7237e
Add basic python structure
sitaowang1998 1f944d0
Fix code structure
sitaowang1998 68992fc
Fix ruff lint
sitaowang1998 1127b22
Extend lint tasks to python directory
sitaowang1998 4c51c91
Add python build tasks
sitaowang1998 56878b2
Merge branch 'main' into mypy_lint
sitaowang1998 bb428c9
Merge branch 'mypy_lint' into tombi
sitaowang1998 944bd5e
Merge branch 'tombi' into build-task
sitaowang1998 d75bc15
Merge branch 'build-task' into python_structure
sitaowang1998 4b7eca2
Merge branch 'main' into tombi
sitaowang1998 8ddbda7
Merge branch 'tombi' into build-task
sitaowang1998 0d998ec
Merge branch 'build-task' into python_structure
sitaowang1998 6004d64
Fix yaml lint error
sitaowang1998 bcab5db
Fix tombi lint
sitaowang1998 a50d25a
Remove wrong mypy config
sitaowang1998 fbebb96
Fix typo and format file
sitaowang1998 0322bd5
Use typed msgpack and remove mypy config for msgpack from pyproject
sitaowang1998 2a08155
Update uv lock
sitaowang1998 0b62f73
Merge branch 'main' into tombi
sitaowang1998 7384158
Merge branch 'main' into build-task
sitaowang1998 70ef90a
Merge branch 'main' into python_structure
sitaowang1998 b09515c
Increase min version of tombi
sitaowang1998 bad9675
Merge branch 'tombi' into build-task
sitaowang1998 e009ff4
Merge branch 'tombi' into python_structure
sitaowang1998 a0407a7
Merge branch 'main' into tombi
sitaowang1998 aa9a8de
Merge branch 'tombi' into build-task
sitaowang1998 3c7e794
Merge branch 'tombi' into python_structure
sitaowang1998 b4d6576
Add uv in README
sitaowang1998 23f31cd
Merge branch 'main' into build-task
sitaowang1998 0167ac0
Merge branch 'build-task' into python_structure
sitaowang1998 f447614
Fix redenduncy caused by merge
sitaowang1998 a84de83
Merge branch 'build-task' into python_structure
sitaowang1998 2d1b6fc
Merge branch 'main' into python_structure
sitaowang1998 061d101
Merge branch 'main' into python_structure
sitaowang1998 de149ee
Add integral types
sitaowang1998 d332dbe
Revert "Add integral types"
sitaowang1998 80b6772
Add package export control for type
sitaowang1998 f2ebaa6
Add floating point type
sitaowang1998 8c27b16
Restructure under src/spider
sitaowang1998 b7f1884
Merge branch 'python_structure' into python_type
sitaowang1998 a7aa6b0
Re-export spider.type under spider
sitaowang1998 95b98c8
Merge branch 'main' into python_structure
sitaowang1998 5765acf
Add tdl types; Allow no docstring for override function
sitaowang1998 427dd6b
Merge branch 'main' into python_structure
sitaowang1998 213d2fc
Add type conversion to tdl type
sitaowang1998 a1a86f3
Add to_tdl_str
sitaowang1998 97af71b
Add to_tdl_type_str to type package export
sitaowang1998 74f31e2
Add pytest and basic test structure
sitaowang1998 7d95706
Add python test tasks
sitaowang1998 96b5324
Rename some cpp tests and add python tests to GH workflow and doc
sitaowang1998 14ea6fb
Don't create __pycache__ when running pytest
sitaowang1998 854cd11
Fix missing link
sitaowang1998 5565c9e
Merge branch 'python_structure' of github.com:sitaowang1998/spider in…
sitaowang1998 2504429
Merge branch 'python_structure' into python_test_setup
sitaowang1998 7ea4928
Use task env
sitaowang1998 01dd0c6
Fix task executor path
sitaowang1998 4cec09b
Merge branch 'python_structure' into python_test_setup
sitaowang1998 e022404
Merge branch 'main' into python_structure
sitaowang1998 8894b90
Merge branch 'main' into python_test_setup
sitaowang1998 7753153
Merge branch 'main' into python_structure
sitaowang1998 dd59915
Merge branch 'main' into python_test_setup
sitaowang1998 ac1d4ee
Merge branch 'python_structure' into python_type
sitaowang1998 0d38c43
Merge branch 'python_test_setup' into python_type
sitaowang1998 438f6e7
Remove unnecessary __init__.py files
sitaowang1998 2caad5d
Merge branch 'python_structure' into python_test_setup
sitaowang1998 c6815ce
Merge branch 'python_test_setup' into python_type
sitaowang1998 61e0651
Fix bugs
sitaowang1998 8b13abd
Add some type convertion tests
sitaowang1998 50b2b25
Bug fix
sitaowang1998 519dcd8
Merge branch 'python_test_setup' into python_type
sitaowang1998 8e2786b
Add more tests
sitaowang1998 a8c8641
Fix pytest
sitaowang1998 9fabc44
Merge branch 'python_test_setup' into python_type
sitaowang1998 e558f67
Improve code according to coderabbit
sitaowang1998 0eea922
Restructure files
sitaowang1998 edb3212
Merge branch 'main' into python_type
sitaowang1998 6eda1bd
Clean up merge
sitaowang1998 dac6d62
Merge branch 'main' into python_type
sitaowang1998 70f2a6b
Merge branch 'main' into python_type
sitaowang1998 e280a79
Merge branch 'main' into python_type
sitaowang1998 0713c0d
Use future annotation and disable Self return type check
sitaowang1998 3b8cfe9
Avoid using chain comparision
sitaowang1998 f3e41f9
Merge branch 'main' into python_type
sitaowang1998 c9cb4b6
Apply suggestions from code review
sitaowang1998 412a2b5
Disable chained boolean comparison in ruff
sitaowang1998 bab4806
Apply suggestions from code review
sitaowang1998 096eda9
Use whether for boolean return type docstring
sitaowang1998 be05c38
Fix BoundedInt
sitaowang1998 5cd5699
Rename private functions
sitaowang1998 bdf1d8f
Fix get_args
sitaowang1998 5602b11
Update python/spider-py/pyproject.toml
sitaowang1998 0696cac
Fix exception docstring
sitaowang1998 d6e32b3
Move native primitive type check inside _to_primitive_tdl_type
sitaowang1998 e916cb9
Use type dict to convert primitive type
sitaowang1998 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1,12 @@ | ||
| """Spider package root.""" | ||
|
|
||
| from spider_py.type import Double, Float, Int8, Int16, Int32, Int64 | ||
|
|
||
| __all__ = [ | ||
| "Double", | ||
| "Float", | ||
| "Int8", | ||
| "Int16", | ||
| "Int32", | ||
| "Int64", | ||
| ] |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| """Spider type package.""" | ||
|
|
||
| from spider_py.type.tdl_convert import to_tdl_type_str | ||
| from spider_py.type.type import Double, Float, Int8, Int16, Int32, Int64 | ||
|
|
||
| __all__ = ["Double", "Float", "Int8", "Int16", "Int32", "Int64", "to_tdl_type_str"] |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,100 @@ | ||
| """Converts native types to TDL types.""" | ||
|
|
||
| from collections.abc import Collection | ||
| from types import GenericAlias | ||
| from typing import get_args, get_origin | ||
|
|
||
| from spider_py.type.tdl_type import ( | ||
| BoolType, | ||
| ClassType, | ||
| DoubleType, | ||
| FloatType, | ||
| Int8Type, | ||
| Int16Type, | ||
| Int32Type, | ||
| Int64Type, | ||
| ListType, | ||
| MapType, | ||
| TdlType, | ||
| ) | ||
| from spider_py.type.type import Double, Float, Int8, Int16, Int32, Int64 | ||
| from spider_py.type.utils import get_class_name | ||
|
|
||
|
|
||
| def _to_primitive_tdl_type(native_type: type | GenericAlias) -> TdlType | None: | ||
| """ | ||
| Converts a native type to primitive TDL type. | ||
| :param native_type: | ||
| :return: | ||
| - The converted TDL primitive if `native_type` is supported. | ||
| - None otherwise. | ||
| """ | ||
| tdl_type: TdlType | None = None | ||
| if native_type is Int8: | ||
| tdl_type = Int8Type() | ||
| elif native_type is Int16: | ||
| tdl_type = Int16Type() | ||
| elif native_type is Int32: | ||
| tdl_type = Int32Type() | ||
| elif native_type is Int64: | ||
| tdl_type = Int64Type() | ||
| elif native_type is Float: | ||
| tdl_type = FloatType() | ||
| elif native_type is Double: | ||
| tdl_type = DoubleType() | ||
| elif native_type is bool: | ||
| tdl_type = BoolType() | ||
| return tdl_type | ||
|
|
||
|
|
||
| def to_tdl_type(native_type: type | GenericAlias) -> TdlType: | ||
| """ | ||
| Converts a Python type to TDL type. | ||
| :param native_type: | ||
| :return: The converted TDL type. | ||
| :raises TypeError: If `native_type` is not a valid TDL type. | ||
| """ | ||
| primitive_tdl_type = _to_primitive_tdl_type(native_type) | ||
| if primitive_tdl_type is not None: | ||
| return primitive_tdl_type | ||
|
|
||
| if native_type in (int, float, str, complex, bytes): | ||
| msg = f"{native_type} is not a valid TDL type." | ||
| raise TypeError(msg) | ||
|
|
||
| if isinstance(native_type, GenericAlias): | ||
sitaowang1998 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| origin = get_origin(native_type) | ||
| if origin is list: | ||
| args = get_args(native_type) | ||
| if len(args) == 0: | ||
| msg = "List does not have an element type." | ||
| raise TypeError(msg) | ||
| arg = args[0] | ||
| return ListType(to_tdl_type(arg)) | ||
|
|
||
| if origin is dict: | ||
| args = get_args(native_type) | ||
| msg = "Dict does not have a key/value type." | ||
| if len(args) != 2: # noqa: PLR2004 | ||
| raise TypeError(msg) | ||
| key = args[0] | ||
| value = args[1] | ||
| return MapType(to_tdl_type(key), to_tdl_type(value)) | ||
|
|
||
| msg = f"{native_type} is not a valid TDL type." | ||
| raise TypeError(msg) | ||
|
|
||
| if issubclass(native_type, Collection): | ||
| msg = f"{native_type} is not a valid TDL type." | ||
| raise TypeError(msg) | ||
|
|
||
| return ClassType(get_class_name(native_type)) | ||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
|
|
||
| def to_tdl_type_str(native_type: type | GenericAlias) -> str: | ||
| """ | ||
| :param native_type: A Python native type. | ||
| :return: A string representation of the TDL type. | ||
| :raises TypeError: If `native_type` is not a valid TDL type. | ||
| """ | ||
| return to_tdl_type(native_type).type_str() | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,144 @@ | ||
| """Spider TDL types.""" | ||
|
|
||
| from abc import ABC, abstractmethod | ||
|
|
||
| from typing_extensions import override | ||
|
|
||
|
|
||
| class TdlType(ABC): | ||
| """Abstract base class for all TDL types.""" | ||
|
|
||
| @abstractmethod | ||
| def type_str(self) -> str: | ||
| """:return: String representation of the TDL type.""" | ||
|
|
||
|
|
||
| class DoubleType(TdlType): | ||
| """TDL double type.""" | ||
|
|
||
| @override | ||
| def type_str(self) -> str: | ||
| return "double" | ||
|
|
||
|
|
||
| class FloatType(TdlType): | ||
| """TDL float type.""" | ||
|
|
||
| @override | ||
| def type_str(self) -> str: | ||
| return "float" | ||
|
|
||
|
|
||
| class Int8Type(TdlType): | ||
| """TDL int8 type.""" | ||
|
|
||
| @override | ||
| def type_str(self) -> str: | ||
| return "int8" | ||
|
|
||
|
|
||
| class Int16Type(TdlType): | ||
| """TDL int16 type.""" | ||
|
|
||
| @override | ||
| def type_str(self) -> str: | ||
| return "int16" | ||
|
|
||
|
|
||
| class Int32Type(TdlType): | ||
| """TDL int32 type.""" | ||
|
|
||
| @override | ||
| def type_str(self) -> str: | ||
| return "int32" | ||
|
|
||
|
|
||
| class Int64Type(TdlType): | ||
| """TDL int64 type.""" | ||
|
|
||
| @override | ||
| def type_str(self) -> str: | ||
| return "int64" | ||
|
|
||
|
|
||
| class BoolType(TdlType): | ||
| """TDL bool type.""" | ||
|
|
||
| @override | ||
| def type_str(self) -> str: | ||
| return "bool" | ||
|
|
||
|
|
||
| class ClassType(TdlType): | ||
| """TDL Custom class type.""" | ||
|
|
||
| def __init__(self, name: str) -> None: | ||
| """ | ||
| Creates a TDL custom class type. | ||
| :param name: The name of the class. | ||
| """ | ||
| self._name = name | ||
|
|
||
| @override | ||
| def type_str(self) -> str: | ||
| return self._name | ||
|
|
||
|
|
||
| class ListType(TdlType): | ||
| """TDL List type.""" | ||
|
|
||
| def __init__(self, element_type: TdlType) -> None: | ||
| """ | ||
| Creates a TDL list type. | ||
| :param element_type: | ||
| """ | ||
| self.element_type = element_type | ||
|
|
||
| @override | ||
| def type_str(self) -> str: | ||
| return f"List<{self.element_type.type_str()}>" | ||
|
|
||
|
|
||
| def is_integral(tdl_type: TdlType) -> bool: | ||
| """ | ||
| :param tdl_type: | ||
| :return: Whether `tdl_type` is a TDL integral type. | ||
| """ | ||
| return isinstance(tdl_type, (Int8Type, Int16Type, Int32Type, Int64Type)) | ||
|
|
||
|
|
||
| def is_string(tdl_type: TdlType) -> bool: | ||
| """ | ||
| :param tdl_type: | ||
| :return: Whether `tdl_type` is a TDL string type, i.e. `List<int8>`. | ||
| """ | ||
| return isinstance(tdl_type, ListType) and isinstance(tdl_type.element_type, Int8Type) | ||
|
|
||
|
|
||
| def is_map_key(tdl_type: TdlType) -> bool: | ||
| """ | ||
| :param tdl_type: | ||
| :return: Whether `tdl_type` is a supported key type of a map. | ||
| """ | ||
| return is_integral(tdl_type) or is_string(tdl_type) | ||
|
|
||
|
|
||
| class MapType(TdlType): | ||
| """TDL Map type.""" | ||
|
|
||
| def __init__(self, key_type: TdlType, value_type: TdlType) -> None: | ||
| """ | ||
| Creates a TDL map type. | ||
sitaowang1998 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| :param key_type: | ||
| :param value_type: | ||
| :raises TypeError: If key is not a supported type. | ||
| """ | ||
| if not is_map_key(key_type): | ||
| msg = f"{key_type} is not a supported type for map key." | ||
| raise TypeError(msg) | ||
| self.key_type = key_type | ||
| self.value_type = value_type | ||
|
|
||
| @override | ||
| def type_str(self) -> str: | ||
| return f"Map<{self.key_type.type_str()},{self.value_type.type_str()}>" | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,74 @@ | ||
| """Custom type module for Spider.""" | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| from typing import cast | ||
|
|
||
|
|
||
| class BoundedInt(int): | ||
| """Bounded integer type.""" | ||
|
|
||
| def __new__(cls, value: int, bits: int = 32) -> BoundedInt: | ||
| """Creates a bounded integer.""" | ||
| if bits not in (8, 16, 32, 64): | ||
| msg = f"Unsupported bits size: {bits}. Supported sizes are 8, 16, 32, or 64." | ||
| raise ValueError(msg) | ||
|
|
||
| lower_bound = -(1 << (bits - 1)) | ||
| upper_bound = (1 << (bits - 1)) - 1 | ||
|
|
||
| if not (lower_bound <= value and value <= upper_bound): | ||
| msg = ( | ||
| f"Bounded integer value ({value}) must be between {lower_bound} and {upper_bound}." | ||
| ) | ||
| raise ValueError(msg) | ||
|
|
||
| return super().__new__(cls, value) | ||
|
|
||
|
|
||
| class Int8(BoundedInt): | ||
| """8 bits integer type.""" | ||
|
|
||
| def __new__(cls, value: int) -> Int8: | ||
| """Creates an int8 integer.""" | ||
| return cast("Int8", super().__new__(cls, value, bits=8)) | ||
|
|
||
|
|
||
| class Int16(BoundedInt): | ||
| """16 bits integer type.""" | ||
|
|
||
| def __new__(cls, value: int) -> Int16: | ||
| """Creates an int16 integer.""" | ||
| return cast("Int16", super().__new__(cls, value, bits=16)) | ||
|
|
||
|
|
||
| class Int32(BoundedInt): | ||
| """32 bits integer type.""" | ||
|
|
||
| def __new__(cls, value: int) -> Int32: | ||
| """Creates an int32 integer.""" | ||
| return cast("Int32", super().__new__(cls, value, bits=32)) | ||
|
|
||
|
|
||
| class Int64(BoundedInt): | ||
| """64 bits integer type.""" | ||
|
|
||
| def __new__(cls, value: int) -> Int64: | ||
| """Creates an int64 integer.""" | ||
| return cast("Int64", super().__new__(cls, value, bits=64)) | ||
|
|
||
|
|
||
| class Float(float): | ||
| """Float type.""" | ||
|
|
||
| def __new__(cls, value: float) -> Float: | ||
| """Creates a float number.""" | ||
| return super().__new__(cls, value) | ||
|
|
||
|
|
||
| class Double(float): | ||
| """Double type.""" | ||
|
|
||
| def __new__(cls, value: float) -> Double: | ||
| """Creates a double number.""" | ||
| return super().__new__(cls, value) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| """Utility for TDL types.""" | ||
|
|
||
|
|
||
| def get_class_name(cls: type) -> str: | ||
| """ | ||
| :param cls: | ||
| :return: Full name of `cls`. | ||
| """ | ||
| return f"{cls.__module__}.{cls.__qualname__}" |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the reason for explicitly listing all these types? Seems like we're also missing some primitive types, for example,
bool.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
boolis the only native Python primitive type that we support. Forintandfloat, we force user to use our own sized type. Forstr, we useList<Int8>.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah that's not too obvious. I'd say this check isn't necessary, considering we already check all the types we support.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The check is necessary. If we don't check for them, the code will reach line 91 and returns a primitive type, against the requirement to throw a
TypeErrorfor unsupported types.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. Then I think this part of code should be refactored a little bit otherwise it's confusing. Can we move this check and throw into
_to_primitive_tdl_typeand make the error message more descriptive, sth like{native_type} is not a TDL type. Please use the corresponding TDL primitive type.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed and pushed.