ENH: Implement pandas.read_iceberg

[datapythonista]

• Tests added and passed if fixing a bug or adding a new feature
• All code checks passed.
• Added type annotations to new arguments/methods/functions.
• Added an entry in the latest doc/source/whatsnew/vX.X.X.rst file if fixing a bug or adding a new feature.

[datapythonista]

There is a test failure, for some reason PyIceberg is able to find the namespace and the table when using:

pyiceberg.catalog.load_catalog(**{"uri": f"sqlite:///{path}/catalog.sqlite"})

but not when using:

pyiceberg.catalog.load_catalog("pandas_tests_catalog")

with config file ~/.pyiceberg:

catalog:
  pandas_tests_catalog:
    type: sql
    uri: sqlite:///path/catalog.sqlite

I need to research more on what's the problem, since this should work. Other than that this should be ready to get merged.

[IsaacWarren]

Hi @datapythonista, I took a look at the failing tests and from what I can tell it looks like writing the .pyicberg.yaml file causes subsequent tests to fail. I'm not really sure why though and I can't reproduce it locally with pyiceberg 0.9. I have a ~/.pyiceberg.yaml file containing

  catalog:
    pandas_tests_catalog:
      uri: sqlite:////tmp/iceberg_catalog/catalog.sqlite

and a test file running

from pyiceberg import catalog

catalog = catalog.load_catalog(None, **{"uri": "sqlite:////tmp/iceberg_catalog/catalog.sqlite"})
print(catalog.list_namespaces())

just fine. I did see that the line removing the ~/.pyiceberg.yaml was commented out, was there a reason for that?

[datapythonista]

Thanks for giving it a try. I just commented that line to make sure locally that it contained what I expected after running the test, I forgot to uncomment.

Passing the uri to load_catalog worked fine locally. I just had problems when passing a catalog name to it (which requires the config file with the uri). I'll have another look, I thought I could be missing something obvious, but I guess it's something more complicated.

[IsaacWarren]

Oh ok, I just tried by passing the catalog name with a config file

from pyiceberg import catalog
from pyiceberg.schema import Schema

catalog = catalog.load_catalog("pandas_tests_catalog")
catalog.create_namespace_if_not_exists("default")
catalog.create_table_if_not_exists("default.test_table", schema=Schema())

and it gave me an error about there not being a default path because warehouse isn't set, maybe try setting warehouse for pandas_tests_catalog in ~/.pyiceberg.yaml? When I added that locally it started working.

[datapythonista]

I tried providing warehouse, but that didn't work.

I've been debugging, and seems like the problem is that whether the catalog is loaded by a URI or by a name, the query that retrieve namespaces is filtering by a catalog name. Which will be default when the catalog is loaded with a URI with no name. I'm still having an issue, after making the name consistent for our tests, but I hope I can get a fix soon.

Thank for the help!

[datapythonista]

Tests should be fixed now. It was a bit tricky, since pyiceberg loads the config when importing the module, not when loading the catalog, which made the config file to not always be loaded.

[datapythonista]

For some reason pyiceberg used to have an upper version for dependencies. They stopped doing it now, but for the minimum pyiceberg version it was not possible to be compatible with all our minimum version dependencies. I relaxed the minimum version of fsspec, s3fs and gcsfs to be able to resolve the minimum versions environment with pyiceberg.

[mroeschke]

Curious if there is an open issue discussing including this new feature.

(FWIW I did like your prior IO registration PDEP that would have made this easier to externally implement)

[datapythonista]

Curious if there is an open issue discussing including this new feature.

I think only the discussions you are aware of, bodo-ai/Bodo-Pandas-Collaboration#9 and the discussions in the calls that I know.

I had a look at using read_sql instead of read_iceberg, and to me it feels like the API would be too difficult to use. Considering the popularity of Iceberg, and that we already have specific connectors for much less popular formats such as feather, SPSS... I found this the most reasonable implementation. But happy to give a try at using the code in this PR with read_sql instead, if there is interest. But it also feels that our code will be more complex, so personally I don't see an advantage.

I'm also happy to revisit PDEP-9. There weren't objections to the general idea that I remember, the main blocker was that some people weren't happy that connectors could register with the name they wanted. And I don't think there is a good solution to this. To me it's not a problem, since at the end is the user who decides which Python dependencies are installed. In any case, to me it makes sense to move forward with this Iceberg, and surely this would be a good candidate to move as a third party with many others if we ever implement PDEP-9.

[mroeschke]

Does it make sense for this to be a pytest fixture? It could be parametrized over different catalog names by default and could use the tmpdir fixture to do the temporary directory stuff automatically

[datapythonista]

That was my first implementation, but I couldn't make a fixture remove the files when the test finish. That's why I implemented it as a context manager.

I'll check again, as I didn't try with the tmpdir fixture, but it may not be easy.

[mroeschke]

Might be good to flag as experimental so if we revisit and implement the IO plugin model we can pivot this to that model a little quicker.

[datapythonista]

Makes sense, thanks for all the feedback.