You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "syun64 (via GitHub)" <gi...@apache.org> on 2024/07/10 21:48:57 UTC
[PR] support Etc/UTC [iceberg-python]
syun64 opened a new pull request, #910:
URL: https://github.com/apache/iceberg-python/pull/910
This ended up being a lot simpler than I had thought it would be.
"Etc/UTC" is just another representation of UTC, and should be safe to support. This will allow us to support arrow dataframes produced with datetimes from libraries like pendulum that use this timezone.
Fixes: https://github.com/apache/iceberg-python/issues/863
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] support PyArrow timestamptz with Etc/UTC [iceberg-python]
Posted by "syun64 (via GitHub)" <gi...@apache.org>.
syun64 commented on code in PR #910:
URL: https://github.com/apache/iceberg-python/pull/910#discussion_r1674691317
##########
pyiceberg/io/pyarrow.py:
##########
@@ -1320,7 +1321,16 @@ def _cast_if_needed(self, field: NestedField, values: pa.Array) -> pa.Array:
and pa.types.is_timestamp(values.type)
and values.type.unit == "ns"
):
- return values.cast(target_type, safe=False)
+ if target_type.tz == "UTC" and values.type.tz in UTC_ALIASES or not target_type.tz and not values.type.tz:
+ return values.cast(target_type, safe=False)
+ if (
+ pa.types.is_timestamp(target_type)
+ and target_type.unit == "us"
+ and pa.types.is_timestamp(values.type)
+ and values.type.unit in {"s", "ms", "us"}
+ ):
+ if target_type.tz == "UTC" and values.type.tz in UTC_ALIASES or not target_type.tz and not values.type.tz:
Review Comment:
Added
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] support PyArrow timestamptz with Etc/UTC [iceberg-python]
Posted by "HonahX (via GitHub)" <gi...@apache.org>.
HonahX commented on PR #910:
URL: https://github.com/apache/iceberg-python/pull/910#issuecomment-2226229012
Merged! Thanks for the great work from @syun64 and reviews from @Fokko @kevinjqliu
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] support PyArrow timestamptz with Etc/UTC [iceberg-python]
Posted by "syun64 (via GitHub)" <gi...@apache.org>.
syun64 commented on code in PR #910:
URL: https://github.com/apache/iceberg-python/pull/910#discussion_r1674913965
##########
tests/io/test_pyarrow.py:
##########
@@ -1798,3 +1799,35 @@ def test_identity_partition_on_multi_columns() -> None:
("n_legs", "ascending"),
("animal", "ascending"),
]) == arrow_table.sort_by([("born_year", "ascending"), ("n_legs", "ascending"), ("animal", "ascending")])
+
+
+def test_to_requested_schema_timestamps(
Review Comment:
I've made the function private on this PR, and have another one up to port back to public API function with the previous signature with a deprecation notice here: https://github.com/apache/iceberg-python/pull/918
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] support PyArrow timestamptz with Etc/UTC [iceberg-python]
Posted by "syun64 (via GitHub)" <gi...@apache.org>.
syun64 commented on code in PR #910:
URL: https://github.com/apache/iceberg-python/pull/910#discussion_r1674351468
##########
pyiceberg/io/pyarrow.py:
##########
@@ -1320,7 +1321,16 @@ def _cast_if_needed(self, field: NestedField, values: pa.Array) -> pa.Array:
and pa.types.is_timestamp(values.type)
and values.type.unit == "ns"
):
- return values.cast(target_type, safe=False)
+ if target_type.tz == "UTC" and values.type.tz in UTC_ALIASES or not target_type.tz and not values.type.tz:
+ return values.cast(target_type, safe=False)
+ if (
+ pa.types.is_timestamp(target_type)
+ and target_type.unit == "us"
+ and pa.types.is_timestamp(values.type)
+ and values.type.unit in {"s", "ms", "us"}
+ ):
+ if target_type.tz == "UTC" and values.type.tz in UTC_ALIASES or not target_type.tz and not values.type.tz:
+ return values.cast(target_type)
Review Comment:
Does this look correct that we are casting the types in order to follow the Iceberg Spec for Parquet Physical and Logical Types? https://iceberg.apache.org/spec/#parquet
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] support PyArrow timestamptz with Etc/UTC [iceberg-python]
Posted by "HonahX (via GitHub)" <gi...@apache.org>.
HonahX merged PR #910:
URL: https://github.com/apache/iceberg-python/pull/910
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] support PyArrow timestamptz with Etc/UTC [iceberg-python]
Posted by "syun64 (via GitHub)" <gi...@apache.org>.
syun64 commented on code in PR #910:
URL: https://github.com/apache/iceberg-python/pull/910#discussion_r1674334862
##########
pyiceberg/table/__init__.py:
##########
@@ -528,10 +528,6 @@ def append(self, df: pa.Table, snapshot_properties: Dict[str, str] = EMPTY_DICT)
)
_check_schema_compatible(self._table.schema(), other_schema=df.schema)
- # cast if the two schemas are compatible but not equal
- table_arrow_schema = self._table.schema().as_arrow()
- if table_arrow_schema != df.schema:
- df = df.cast(table_arrow_schema)
Review Comment:
Removed this cast function - `to_requested_schema` should be responsible for casting the types to their desired schema, instead of casting it here
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] support PyArrow timestamptz with Etc/UTC [iceberg-python]
Posted by "syun64 (via GitHub)" <gi...@apache.org>.
syun64 commented on code in PR #910:
URL: https://github.com/apache/iceberg-python/pull/910#discussion_r1674689843
##########
pyiceberg/io/pyarrow.py:
##########
@@ -1320,7 +1321,16 @@ def _cast_if_needed(self, field: NestedField, values: pa.Array) -> pa.Array:
and pa.types.is_timestamp(values.type)
and values.type.unit == "ns"
):
- return values.cast(target_type, safe=False)
+ if target_type.tz == "UTC" and values.type.tz in UTC_ALIASES or not target_type.tz and not values.type.tz:
+ return values.cast(target_type, safe=False)
+ if (
+ pa.types.is_timestamp(target_type)
+ and target_type.unit == "us"
+ and pa.types.is_timestamp(values.type)
+ and values.type.unit in {"s", "ms", "us"}
Review Comment:
Sorry this is a bit confusing, I agree 😓 . I included it here because I wanted to support casting `pa.timestamp('us', tz='Etc/UTC')` to `pa.timestamp('us', tz='UTC')` within the same condition.
I think we won't hit this condition if both the input and requested types are `pa.timestamp('us')` because we enter this block only if target_type and values.type are not equal:
https://github.com/apache/iceberg-python/blob/main/pyiceberg/io/pyarrow.py#L1315
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] support PyArrow timestamptz with Etc/UTC [iceberg-python]
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on code in PR #910:
URL: https://github.com/apache/iceberg-python/pull/910#discussion_r1674710445
##########
pyiceberg/io/pyarrow.py:
##########
@@ -1320,7 +1321,16 @@ def _cast_if_needed(self, field: NestedField, values: pa.Array) -> pa.Array:
and pa.types.is_timestamp(values.type)
and values.type.unit == "ns"
):
- return values.cast(target_type, safe=False)
+ if target_type.tz == "UTC" and values.type.tz in UTC_ALIASES or not target_type.tz and not values.type.tz:
+ return values.cast(target_type, safe=False)
+ if (
+ pa.types.is_timestamp(target_type)
+ and target_type.unit == "us"
+ and pa.types.is_timestamp(values.type)
+ and values.type.unit in {"s", "ms", "us"}
Review Comment:
I think we might need to do some more work here. In Iceberg we're rather strict on the distinction between Timestamp and TimestampTZ. A good way of showing this can be found here:
https://github.com/apache/iceberg-python/blob/aceed2ad9898eaa8b0cdddd883a195e58af5d0e3/pyiceberg/expressions/literals.py#L566-L572
This is when we parse a string from a literal, which often comes an expression: `dt >= '1925-05-22T00:00:00'`. If the value has a timezone, even UTC, we reject it as `Timestamp`. When it has a timestamp, we [normalize it to UTC and then store the integer](https://github.com/apache/iceberg-python/blob/aceed2ad9898eaa8b0cdddd883a195e58af5d0e3/pyiceberg/utils/datetime.py#L75).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] support PyArrow timestamptz with Etc/UTC [iceberg-python]
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on code in PR #910:
URL: https://github.com/apache/iceberg-python/pull/910#discussion_r1674684198
##########
pyiceberg/io/pyarrow.py:
##########
@@ -1320,7 +1321,16 @@ def _cast_if_needed(self, field: NestedField, values: pa.Array) -> pa.Array:
and pa.types.is_timestamp(values.type)
and values.type.unit == "ns"
):
- return values.cast(target_type, safe=False)
+ if target_type.tz == "UTC" and values.type.tz in UTC_ALIASES or not target_type.tz and not values.type.tz:
+ return values.cast(target_type, safe=False)
+ if (
+ pa.types.is_timestamp(target_type)
+ and target_type.unit == "us"
+ and pa.types.is_timestamp(values.type)
+ and values.type.unit in {"s", "ms", "us"}
+ ):
+ if target_type.tz == "UTC" and values.type.tz in UTC_ALIASES or not target_type.tz and not values.type.tz:
Review Comment:
Can we add parentheses?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] support PyArrow timestamptz with Etc/UTC [iceberg-python]
Posted by "syun64 (via GitHub)" <gi...@apache.org>.
syun64 commented on code in PR #910:
URL: https://github.com/apache/iceberg-python/pull/910#discussion_r1674797591
##########
pyiceberg/io/pyarrow.py:
##########
@@ -1296,31 +1297,49 @@ def to_requested_schema(
class ArrowProjectionVisitor(SchemaWithPartnerVisitor[pa.Array, Optional[pa.Array]]):
- file_schema: Schema
+ _file_schema: Schema
_include_field_ids: bool
+ _downcast_ns_timestamp_to_us: bool
def __init__(self, file_schema: Schema, downcast_ns_timestamp_to_us: bool = False, include_field_ids: bool = False) -> None:
- self.file_schema = file_schema
+ self._file_schema = file_schema
self._include_field_ids = include_field_ids
- self.downcast_ns_timestamp_to_us = downcast_ns_timestamp_to_us
+ self._downcast_ns_timestamp_to_us = downcast_ns_timestamp_to_us
def _cast_if_needed(self, field: NestedField, values: pa.Array) -> pa.Array:
- file_field = self.file_schema.find_field(field.field_id)
+ file_field = self._file_schema.find_field(field.field_id)
if field.field_type.is_primitive:
if field.field_type != file_field.field_type:
return values.cast(
schema_to_pyarrow(promote(file_field.field_type, field.field_type), include_field_ids=self._include_field_ids)
)
elif (target_type := schema_to_pyarrow(field.field_type, include_field_ids=self._include_field_ids)) != values.type:
- # Downcasting of nanoseconds to microseconds
- if (
- pa.types.is_timestamp(target_type)
- and target_type.unit == "us"
- and pa.types.is_timestamp(values.type)
- and values.type.unit == "ns"
- ):
- return values.cast(target_type, safe=False)
+ if field.field_type == TimestampType():
Review Comment:
stricter and clearer `field_type` driven checks
##########
tests/io/test_pyarrow.py:
##########
@@ -1798,3 +1799,35 @@ def test_identity_partition_on_multi_columns() -> None:
("n_legs", "ascending"),
("animal", "ascending"),
]) == arrow_table.sort_by([("born_year", "ascending"), ("n_legs", "ascending"), ("animal", "ascending")])
+
+
+def test_to_requested_schema_timestamps(
Review Comment:
I noticed that there weren't any tests for `to_requested_schema` although it is a public API, so I added this in.
Do we want to keep `to_requested_schema` as a public API?
##########
pyiceberg/io/pyarrow.py:
##########
@@ -1320,7 +1321,16 @@ def _cast_if_needed(self, field: NestedField, values: pa.Array) -> pa.Array:
and pa.types.is_timestamp(values.type)
and values.type.unit == "ns"
):
- return values.cast(target_type, safe=False)
+ if target_type.tz == "UTC" and values.type.tz in UTC_ALIASES or not target_type.tz and not values.type.tz:
+ return values.cast(target_type, safe=False)
+ if (
+ pa.types.is_timestamp(target_type)
+ and target_type.unit == "us"
+ and pa.types.is_timestamp(values.type)
+ and values.type.unit in {"s", "ms", "us"}
Review Comment:
Sounds good @Fokko- thank you for the review.
I've made these checks stricter and also clearer for users to follow.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] support PyArrow timestamptz with Etc/UTC [iceberg-python]
Posted by "HonahX (via GitHub)" <gi...@apache.org>.
HonahX commented on code in PR #910:
URL: https://github.com/apache/iceberg-python/pull/910#discussion_r1673466255
##########
pyiceberg/io/pyarrow.py:
##########
@@ -937,7 +937,7 @@ def primitive(self, primitive: pa.DataType) -> PrimitiveType:
else:
raise TypeError(f"Unsupported precision for timestamp type: {primitive.unit}")
- if primitive.tz == "UTC" or primitive.tz == "+00:00":
+ if primitive.tz in ("UTC", "+00:00", "Etc/UTC"):
Review Comment:
In the future, shall we consider add other representations of UTC, such as `"Z"`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] support PyArrow timestamptz with Etc/UTC [iceberg-python]
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on code in PR #910:
URL: https://github.com/apache/iceberg-python/pull/910#discussion_r1673737827
##########
pyiceberg/io/pyarrow.py:
##########
@@ -937,7 +937,7 @@ def primitive(self, primitive: pa.DataType) -> PrimitiveType:
else:
raise TypeError(f"Unsupported precision for timestamp type: {primitive.unit}")
- if primitive.tz == "UTC" or primitive.tz == "+00:00":
+ if primitive.tz in ("UTC", "+00:00", "Etc/UTC"):
Review Comment:
Supernit:
```suggestion
if primitive.tz in {"UTC", "+00:00", "Etc/UTC"}:
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] support PyArrow timestamptz with Etc/UTC [iceberg-python]
Posted by "syun64 (via GitHub)" <gi...@apache.org>.
syun64 commented on PR #910:
URL: https://github.com/apache/iceberg-python/pull/910#issuecomment-2223408958
@Fokko @HonahX - thank you for your reviews. I've updated the integration test to make the [check more comprehensive](https://github.com/apache/iceberg-python/pull/910/files#diff-7f3dd1244d08ce27c003cd091da10aa049f7bb0c7d5397acb4ec69767036accdR1063-R1074), and removed the [pyarrow table casting](https://github.com/apache/iceberg-python/pull/910/files#diff-23e8153e0fd497a9212215bd2067068f3b56fa071770c7ef326db3d3d03cee9bL590-L593) in `append` and `overwrite` so that each column can be cast with more targeted `safe` flag for each field.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] support PyArrow timestamptz with Etc/UTC [iceberg-python]
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on code in PR #910:
URL: https://github.com/apache/iceberg-python/pull/910#discussion_r1674682639
##########
pyiceberg/io/pyarrow.py:
##########
@@ -1320,7 +1321,16 @@ def _cast_if_needed(self, field: NestedField, values: pa.Array) -> pa.Array:
and pa.types.is_timestamp(values.type)
and values.type.unit == "ns"
):
- return values.cast(target_type, safe=False)
+ if target_type.tz == "UTC" and values.type.tz in UTC_ALIASES or not target_type.tz and not values.type.tz:
+ return values.cast(target_type, safe=False)
+ if (
+ pa.types.is_timestamp(target_type)
+ and target_type.unit == "us"
+ and pa.types.is_timestamp(values.type)
+ and values.type.unit in {"s", "ms", "us"}
+ ):
+ if target_type.tz == "UTC" and values.type.tz in UTC_ALIASES or not target_type.tz and not values.type.tz:
+ return values.cast(target_type)
Review Comment:
Yes, it looks good. Arrow writes `INT64` by default for timestamps: https://arrow.apache.org/docs/cpp/parquet.html#logical-types
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] support PyArrow timestamptz with Etc/UTC [iceberg-python]
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on code in PR #910:
URL: https://github.com/apache/iceberg-python/pull/910#discussion_r1674686353
##########
pyiceberg/io/pyarrow.py:
##########
@@ -1320,7 +1321,16 @@ def _cast_if_needed(self, field: NestedField, values: pa.Array) -> pa.Array:
and pa.types.is_timestamp(values.type)
and values.type.unit == "ns"
):
- return values.cast(target_type, safe=False)
+ if target_type.tz == "UTC" and values.type.tz in UTC_ALIASES or not target_type.tz and not values.type.tz:
+ return values.cast(target_type, safe=False)
+ if (
+ pa.types.is_timestamp(target_type)
+ and target_type.unit == "us"
+ and pa.types.is_timestamp(values.type)
+ and values.type.unit in {"s", "ms", "us"}
Review Comment:
When we're requesting `us`and the file provide a `us`, I don't think we need to cast?
```suggestion
and values.type.unit in {"s", "ms"}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] support PyArrow timestamptz with Etc/UTC [iceberg-python]
Posted by "HonahX (via GitHub)" <gi...@apache.org>.
HonahX commented on code in PR #910:
URL: https://github.com/apache/iceberg-python/pull/910#discussion_r1674879351
##########
tests/io/test_pyarrow.py:
##########
@@ -1798,3 +1799,35 @@ def test_identity_partition_on_multi_columns() -> None:
("n_legs", "ascending"),
("animal", "ascending"),
]) == arrow_table.sort_by([("born_year", "ascending"), ("n_legs", "ascending"), ("animal", "ascending")])
+
+
+def test_to_requested_schema_timestamps(
Review Comment:
+1 on making `to_requested_schema` private. IMO this seem to be specific to our internal workflow so making it private can give us more flexibility in future development.
Actually, the signature of this API has been updated in https://github.com/apache/iceberg-python/pull/786: `pa.Table` to `pa.RecordBatch`, which is a breaking change. I think it may be a good time to send a deprecation notice on the old one and making the new one private. WDYT?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org