SNOW-1794355: [API Coverage] StructType #2623
Conversation
CHANGELOG.md
Outdated
| @@ -11,6 +11,11 @@ | |||
| - `simple_string` | |||
| - `json_value` | |||
| - `json` | |||
| - Added support for method `from_json` in the following type classes: | |||
There was a problem hiding this comment.
we could possible combine all types related improvement summary in a common changelog.
There was a problem hiding this comment.
do you mean I should add all of them under ##### Types ?
There was a problem hiding this comment.
something like
- Added new methods to enhance data type handling and JSON serialization/deserialization:
- To `DataType`, its derived classes, and `StructField`:
- `type_name`: Returns the type name of the data.
- `simple_string`: Provides a simple string representation of the data.
- `json_value`: Returns the data as a JSON-compatible value.
- `json`: Converts the data to a JSON string.
- To `ArrayType`, `MapType`, `StructField`, and `StructType`:
- `from_json`: Enables these types to be created from JSON data.
There was a problem hiding this comment.
sure, will do
src/snowflake/snowpark/types.py
Outdated
| _FIXED_DECIMAL = re.compile(r"decimal\(\s*(\d+)\s*,\s*(-?\d+)\s*\)") | ||
|
|
There was a problem hiding this comment.
| _FIXED_DECIMAL = re.compile(r"decimal\(\s*(\d+)\s*,\s*(-?\d+)\s*\)") | |
| _FIXED_DECIMAL = re.compile(r"decimal\(\s*(\d+)\s*,\s*(\d+)\s*\)") | |
we shouldn't be expecting negative numbers right?
src/snowflake/snowpark/types.py
Outdated
| v.typeName(): v for v in _complex_types | ||
| } | ||
|
|
||
| _FIXED_DECIMAL = re.compile(r"decimal\(\s*(\d+)\s*,\s*(-?\d+)\s*\)") |
There was a problem hiding this comment.
| _FIXED_DECIMAL = re.compile(r"decimal\(\s*(\d+)\s*,\s*(-?\d+)\s*\)") | |
| _FIXED_DECIMAL_PATTERN = re.compile(r"decimal\(\s*(\d+)\s*,\s*(-?\d+)\s*\)") |
tests/unit/test_types.py
Outdated
| def test_structtype_from_json(tpe, json_dict, expected_result): | ||
| result = tpe.from_json(json_dict) | ||
| assert result == expected_result | ||
| assert isinstance(result, tpe) |
There was a problem hiding this comment.
I'm wondering if we can write some round-trip tests instead of re-writing a full set.
something like
datatype = MapType(IntegerType(), StringType())
json_value = datatype.jsonValue()
assert datatype == MapType.fromJson(json_value)
There was a problem hiding this comment.
i changed the test so that it is more concise
There was a problem hiding this comment.
thank you! looks nice.
|
Could you please also fill the description so future readers have more context. |
Which Jira issue is this PR addressing? Make sure that there is an accompanying issue to your PR.
Fixes SNOW-1794355
Fill out the following pre-review checklist:
Please describe how your code solves the related issue.
This PR meant to add from_json() method to StructType and other type classes that need it(ArrayType, MapType).
All the toInternal and fromInternal methods are not added because they are specific to pyspark to covert python object to/from internal java spark object