sungwy commented on code in PR #1669:
URL: https://github.com/apache/iceberg-python/pull/1669#discussion_r1985005690
##########
pyiceberg/io/pyarrow.py:
##########
@@ -1655,19 +1646,16 @@ class
ArrowProjectionVisitor(SchemaWithPartnerVisitor[pa.Array, Optional[pa.Arra
_file_schema: Schema
_include_field_ids: bool
_downcast_ns_timestamp_to_us: bool
- _use_large_types: bool
def __init__(
self,
file_schema: Schema,
downcast_ns_timestamp_to_us: bool = False,
include_field_ids: bool = False,
- use_large_types: bool = True,
Review Comment:
I agree that it may be more confusing @Fokko ! Regretfully I admit that the
way this config property was introduced was not forward thinking of when we'd
plan to remove its support.
I still think that changing the default behavior makes it probable for a
minor version upgrade to lead to an issue, as 'string' and 'large_string' do
not play nicely in PyArrow. Should we just leave the default value as is, and
keep the deprecation message, so that users who are relying on the current
default value are made aware of the change in behavior in the next minor
release? This will be noisy, but I think it would at least be better than
changing the type of most users without prior notice.
Let me know your thoughts!
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
