raulcd commented on issue #2743:
URL:
https://github.com/apache/iceberg-python/issues/2743#issuecomment-3523619899
@kevinjqliu I plan to work on this one, can you assign it to me please?
I have tried a couple of approaches, we could do the following which isn't
great because we lose some typing annotations for our test signatures but we
could define when retrieving the fixture:
```diff
@@ -193,17 +192,20 @@ def
test_create_table_removes_trailing_slash_from_location(catalog: InMemoryCata
@pytest.mark.parametrize(
"schema,expected",
[
- (lazy_fixture("pyarrow_schema_simple_without_ids"),
lazy_fixture("iceberg_schema_simple_no_ids")),
- (lazy_fixture("iceberg_schema_simple"),
lazy_fixture("iceberg_schema_simple")),
- (lazy_fixture("iceberg_schema_nested"),
lazy_fixture("iceberg_schema_nested")),
- (lazy_fixture("pyarrow_schema_nested_without_ids"),
lazy_fixture("iceberg_schema_nested_no_ids")),
+ ("pyarrow_schema_simple_without_ids",
"iceberg_schema_simple_no_ids"),
+ ("iceberg_schema_simple", "iceberg_schema_simple"),
+ ("iceberg_schema_nested", "iceberg_schema_nested"),
+ ("pyarrow_schema_nested_without_ids",
"iceberg_schema_nested_no_ids"),
],
)
def test_convert_schema_if_needed(
schema: str,
expected: str,
catalog: InMemoryCatalog,
+ request: pytest.FixtureRequest,
) -> None:
+ schema: Schema | pa.Schema, = request.getfixturevalue(schema)
+ expected: Schema = request.getfixturevalue(expected)
assert expected == catalog._convert_schema_if_needed(schema)
```
or we could create interim fixtures to maintain signatures, even though we
create some functions that seem unnecessary
```diff
[email protected]
+def catalog(request: pytest.FixtureRequest) -> SqlCatalog:
+ """Indirect fixture for catalog parametrization."""
+ return request.getfixturevalue(request.param)
+
+
[email protected]
+def table_identifier(request: pytest.FixtureRequest) -> Identifier:
+ """Indirect fixture for table_identifier parametrization."""
+ return request.getfixturevalue(request.param)
+
+
[email protected]
+def namespace(request: pytest.FixtureRequest) -> tuple[str, ...]:
+ """Indirect fixture for namespace parametrization."""
+ return request.getfixturevalue(request.param)
+
+
[email protected]
+def from_table_identifier(request: pytest.FixtureRequest) -> Identifier:
+ """Indirect fixture for from_table_identifier parametrization."""
+ return request.getfixturevalue(request.param)
+
+
[email protected]
+def to_table_identifier(request: pytest.FixtureRequest) -> Identifier:
+ """Indirect fixture for to_table_identifier parametrization."""
+ return request.getfixturevalue(request.param)
+
+
[email protected]
+def table_identifier_1(request: pytest.FixtureRequest) -> Identifier:
+ """Indirect fixture for table_identifier_1 parametrization."""
+ return request.getfixturevalue(request.param)
+
+
[email protected]
+def table_identifier_2(request: pytest.FixtureRequest) -> Identifier:
+ """Indirect fixture for table_identifier_2 parametrization."""
+ return request.getfixturevalue(request.param)
+
+
def test_creation_with_no_uri(catalog_name: str) -> None:
with pytest.raises(NoSuchPropertyException):
SqlCatalog(catalog_name, not_uri="unused")
@@ -293,9 +334,10 @@ def
test_creation_when_all_tables_exists(alchemy_engine: Engine, catalog_name: s
@pytest.mark.parametrize(
"catalog",
[
- lazy_fixture("catalog_memory"),
- lazy_fixture("catalog_sqlite"),
+ "catalog_memory",
+ "catalog_sqlite",
],
+ indirect=True,
)
def test_create_tables_idempotency(catalog: SqlCatalog) -> None:
# Second initialization should not fail even if tables are already
created
@@ -306,16 +348,18 @@ def test_create_tables_idempotency(catalog:
SqlCatalog) -> None:
@pytest.mark.parametrize(
"catalog",
[
- lazy_fixture("catalog_memory"),
- lazy_fixture("catalog_sqlite"),
+ "catalog_memory",
+ "catalog_sqlite",
],
+ indirect=True,
)
@pytest.mark.parametrize(
"table_identifier",
[
- lazy_fixture("random_table_identifier"),
- lazy_fixture("random_hierarchical_identifier"),
+ "random_table_identifier",
+ "random_hierarchical_identifier",
],
+ indirect=True,
)
def test_create_table_default_sort_order(catalog: SqlCatalog,
table_schema_nested: Schema, table_identifier: Identifier) -> None:
namespace = Catalog.namespace_from(table_identifier)
```
Which approach do you prefer.
Based on some investigation it seems the second approach is the recommended
one.
--
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]
