You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "rambleraptor (via GitHub)" <gi...@apache.org> on 2025/08/08 22:04:43 UTC
[PR] Add the rest of the _convert_schema_if_needed calls [iceberg-python]
rambleraptor opened a new pull request, #2300:
URL: https://github.com/apache/iceberg-python/pull/2300
<!--
Thanks for opening a pull request!
-->
Closes #2270
This adds the rest of the _convert_schema_if_needed calls.
# Rationale for this change
# Are these changes tested?
# Are there any user-facing changes?
<!-- In the case of user-facing changes, please add the changelog label. -->
--
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] Add the rest of the _convert_schema_if_needed calls [iceberg-python]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu merged PR #2300:
URL: https://github.com/apache/iceberg-python/pull/2300
--
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] Add the rest of the _convert_schema_if_needed calls [iceberg-python]
Posted by "rambleraptor (via GitHub)" <gi...@apache.org>.
rambleraptor commented on PR #2300:
URL: https://github.com/apache/iceberg-python/pull/2300#issuecomment-3169484638
good call. both instances fixed
--
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] Add the rest of the _convert_schema_if_needed calls [iceberg-python]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on PR #2300:
URL: https://github.com/apache/iceberg-python/pull/2300#issuecomment-3199132579
looks like the linter errored
```
pyiceberg/table/update/schema.py:146: error: Incompatible default for argument "format_version" (default has type "Literal['TableProperties.DEFAULT_FORMAT_VERSION']", argument has type "Literal[1, 2, 3]") [assignment]
```
--
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] Add the rest of the _convert_schema_if_needed calls [iceberg-python]
Posted by "rambleraptor (via GitHub)" <gi...@apache.org>.
rambleraptor commented on PR #2300:
URL: https://github.com/apache/iceberg-python/pull/2300#issuecomment-3199356645
Ugh, circular imports! I made a fix with a TODO. The proper solution is to move TableProperties to a separate module. Happy to do that, but that feels bigger than this PR.
--
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] Add the rest of the _convert_schema_if_needed calls [iceberg-python]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on code in PR #2300:
URL: https://github.com/apache/iceberg-python/pull/2300#discussion_r2264851987
##########
tests/integration/test_writes/test_writes.py:
##########
@@ -2251,13 +2251,19 @@ def test_branch_py_write_spark_read(session_catalog: Catalog, spark: SparkSessio
@pytest.mark.integration
def test_nanosecond_support_on_catalog(session_catalog: Catalog) -> None:
identifier = "default.test_nanosecond_support_on_catalog"
- # Create a pyarrow table with a nanosecond timestamp column
- table = pa.Table.from_arrays(
- [
- pa.array([datetime.now()], type=pa.timestamp("ns")),
- pa.array([datetime.now()], type=pa.timestamp("ns", tz="America/New_York")),
- ],
- names=["timestamp_ns", "timestamptz_ns"],
+
+ catalog = load_catalog("default", type="in-memory")
Review Comment:
nit can we use `arrow_table_schema_with_nanoseconds_timestamp_precisions` here? its pretty comprehensive
##########
tests/integration/test_writes/test_writes.py:
##########
@@ -2251,13 +2251,19 @@ def test_branch_py_write_spark_read(session_catalog: Catalog, spark: SparkSessio
@pytest.mark.integration
def test_nanosecond_support_on_catalog(session_catalog: Catalog) -> None:
identifier = "default.test_nanosecond_support_on_catalog"
- # Create a pyarrow table with a nanosecond timestamp column
- table = pa.Table.from_arrays(
- [
- pa.array([datetime.now()], type=pa.timestamp("ns")),
- pa.array([datetime.now()], type=pa.timestamp("ns", tz="America/New_York")),
- ],
- names=["timestamp_ns", "timestamptz_ns"],
+
+ catalog = load_catalog("default", type="in-memory")
+ catalog.create_namespace("ns")
+
+ table = pa.Table.from_arrays([pa.array([datetime.now()], type=pa.timestamp("ns"))], names=["timestamps"])
+ table2 = pa.Table.from_arrays(
+ [pa.array([datetime.now()], type=pa.timestamp("ns", tz="America/New_York"))], names=["timestamps"]
)
_create_table(session_catalog, identifier, {"format-version": "3"}, schema=table.schema)
+
+ with pytest.raises(NotImplementedError, match="Writing V3 is not yet supported"):
+ catalog.create_table("ns.table1", schema=table.schema, properties={"format-version": "3"})
+
+ with pytest.raises(NotImplementedError, match="Writing V3 is not yet supported"):
+ catalog.create_table("ns.table2", schema=table2.schema, properties={"format-version": "3"})
Review Comment:
nit can we test that the same code that works for v3 doesnt work for v2. something like
```
with pytest.raises(NotImplementedError, match="..."):
_create_table(session_catalog, identifier, {"format-version": "2"}, schema=table.schema)
```
--
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] Add the rest of the _convert_schema_if_needed calls [iceberg-python]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on PR #2300:
URL: https://github.com/apache/iceberg-python/pull/2300#issuecomment-3200364705
> The proper solution is to move TableProperties to a separate module
yea makes sense we can follow up and fix the TODO
--
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] Add the rest of the _convert_schema_if_needed calls [iceberg-python]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on PR #2300:
URL: https://github.com/apache/iceberg-python/pull/2300#issuecomment-3200379051
Thanks for the PR @rambleraptor
--
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] Add the rest of the _convert_schema_if_needed calls [iceberg-python]
Posted by "rambleraptor (via GitHub)" <gi...@apache.org>.
rambleraptor commented on PR #2300:
URL: https://github.com/apache/iceberg-python/pull/2300#issuecomment-3197885707
Hey @kevinjqliu, sorry this took so long! I was OOO (literally on a boat with poor WiFi...), so I'm just getting to this now. Let me know if there's anything else you need!
--
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