Remove deprecated lazy_fixture dependency

[raulcd]

Feature Request / Improvement

We heavy relay on pytest-lazy-fixture for the project:

Unfortunately lazy-fixture hasn't been maintained (not a commit to the project over the last 3 years) and since pytest==8.0.0 (released on 2024-01-27), lazy-fixture is not compatible, see: https://github.com/TvoroG/pytest-lazy-fixture/issues/65

This means we are currently stuck with pytest 7.4.4:

• https://github.com/apache/iceberg-python/pull/2727

currently pytest latest is 9.0.1: https://github.com/pytest-dev/pytest/releases

We should remove our usage of pytest-lazy-fixture and once we have done so we should upgrade pytest, we can do that separately.

[raulcd]

@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:

@@ -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

[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.

[raulcd]

@Fokko FYI about the different approaches. I might go with the second but as there are hundreds of uses of lazy-fixture I was trying to get an agreement of the approach before having to go back and change all of them again :)

[kevinjqliu]

hey @raulcd thanks for working on this.

i like the second approach. It keeps the existing structure where we can define the fixtures once in conftest.py and reuse them in the function decorators

[Fokko]

Nice one @raulcd, I also prefer the second approach. Thanks for working on this 🙌