From 948f8f6da93d547703582ba2afd25766843e8a2a Mon Sep 17 00:00:00 2001 From: Kevin Liu Date: Wed, 6 Mar 2024 19:03:10 -0800 Subject: [PATCH 1/2] pass dict properties to set_properties --- pyiceberg/table/__init__.py | 8 ++++++-- tests/integration/test_reads.py | 36 +++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/pyiceberg/table/__init__.py b/pyiceberg/table/__init__.py index 1a4183c914..13e1ac1da5 100644 --- a/pyiceberg/table/__init__.py +++ b/pyiceberg/table/__init__.py @@ -294,17 +294,21 @@ def upgrade_table_version(self, format_version: Literal[1, 2]) -> Transaction: return self - def set_properties(self, **updates: str) -> Transaction: + def set_properties(self, properties: Properties = EMPTY_DICT, **kwargs: str) -> Transaction: """Set properties. When a property is already set, it will be overwritten. Args: - updates: The properties set on the table. + properties: The properties set on the table. + kwargs: properties can also be pass as kwargs. Returns: The alter table builder. """ + if properties and kwargs: + raise ValueError("Cannot pass both properties and kwargs") + updates = properties or kwargs return self._apply((SetPropertiesUpdate(updates=updates),)) def update_schema(self, allow_incompatible_changes: bool = False, case_sensitive: bool = True) -> UpdateSchema: diff --git a/tests/integration/test_reads.py b/tests/integration/test_reads.py index 072fd7db25..645d66ac32 100644 --- a/tests/integration/test_reads.py +++ b/tests/integration/test_reads.py @@ -124,6 +124,42 @@ def test_table_properties(catalog: Catalog) -> None: assert table.properties == DEFAULT_PROPERTIES +@pytest.mark.integration +@pytest.mark.parametrize('catalog', [pytest.lazy_fixture('catalog_hive'), pytest.lazy_fixture('catalog_rest')]) +def test_table_properties_dict(catalog: Catalog) -> None: + table = create_table(catalog) + + assert table.properties == DEFAULT_PROPERTIES + + with table.transaction() as transaction: + transaction.set_properties({"abc": "🤪"}) + + assert table.properties == dict({"abc": "🤪"}, **DEFAULT_PROPERTIES) + + with table.transaction() as transaction: + transaction.remove_properties("abc") + + assert table.properties == DEFAULT_PROPERTIES + + table = table.transaction().set_properties({"abc": "def"}).commit_transaction() + + assert table.properties == dict({"abc": "def"}, **DEFAULT_PROPERTIES) + + table = table.transaction().remove_properties("abc").commit_transaction() + + assert table.properties == DEFAULT_PROPERTIES + + +@pytest.mark.integration +@pytest.mark.parametrize('catalog', [pytest.lazy_fixture('catalog_hive'), pytest.lazy_fixture('catalog_rest')]) +def test_table_properties_error(catalog: Catalog) -> None: + table = create_table(catalog) + properties = {"abc": "def"} + with pytest.raises(ValueError) as e: + table.transaction().set_properties(properties, abc="def").commit_transaction() + assert "Cannot pass both properties and kwargs" in str(e.value) + + @pytest.mark.integration @pytest.mark.parametrize('catalog', [pytest.lazy_fixture('catalog_hive'), pytest.lazy_fixture('catalog_rest')]) def test_pyarrow_nan(catalog: Catalog) -> None: From b2cd2e3044ba164f6a5b6278892b1578b6aee7e8 Mon Sep 17 00:00:00 2001 From: Kevin Liu Date: Wed, 6 Mar 2024 20:25:32 -0800 Subject: [PATCH 2/2] whitespace --- tests/integration/test_reads.py | 8 -------- 1 file changed, 8 deletions(-) diff --git a/tests/integration/test_reads.py b/tests/integration/test_reads.py index 645d66ac32..da43e7825e 100644 --- a/tests/integration/test_reads.py +++ b/tests/integration/test_reads.py @@ -107,20 +107,16 @@ def test_table_properties(catalog: Catalog) -> None: with table.transaction() as transaction: transaction.set_properties(abc="🤪") - assert table.properties == dict(abc="🤪", **DEFAULT_PROPERTIES) with table.transaction() as transaction: transaction.remove_properties("abc") - assert table.properties == DEFAULT_PROPERTIES table = table.transaction().set_properties(abc="def").commit_transaction() - assert table.properties == dict(abc="def", **DEFAULT_PROPERTIES) table = table.transaction().remove_properties("abc").commit_transaction() - assert table.properties == DEFAULT_PROPERTIES @@ -133,20 +129,16 @@ def test_table_properties_dict(catalog: Catalog) -> None: with table.transaction() as transaction: transaction.set_properties({"abc": "🤪"}) - assert table.properties == dict({"abc": "🤪"}, **DEFAULT_PROPERTIES) with table.transaction() as transaction: transaction.remove_properties("abc") - assert table.properties == DEFAULT_PROPERTIES table = table.transaction().set_properties({"abc": "def"}).commit_transaction() - assert table.properties == dict({"abc": "def"}, **DEFAULT_PROPERTIES) table = table.transaction().remove_properties("abc").commit_transaction() - assert table.properties == DEFAULT_PROPERTIES