ENH: Implement pandas.read_iceberg
State: closed

- [X] [Tests added and passed](https://pandas.pydata.org/pandas-docs/dev/development/contributing_codebase.html#writing-tests) if fixing a bug or adding a new feature
- [X] All [code checks passed](https://pandas.pydata.org/pandas-docs/dev/development/contributing_codebase.html#pre-commit).
- [X] Added [type annotations](https://pandas.pydata.org/pandas-docs/dev/development/contributing_codebase.html#type-hints) to new arguments/methods/functions.
- [X] Added an entry in the latest `doc/source/whatsnew/vX.X.X.rst` file if fixing a bug or adding a new feature.


Labels:
name: IO Data
description: IO issues that don't fit into a more specific label

Comments:
There is a test failure, for some reason PyIceberg is able to find the namespace and the table when using:

```python
pyiceberg.catalog.load_catalog(**{"uri": f"sqlite:///{path}/catalog.sqlite"})
```

but not when using:

```python
pyiceberg.catalog.load_catalog("pandas_tests_catalog")
```

with config file `~/.pyiceberg`:

```yaml
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.
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
```python
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?
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.
Oh ok, I just tried by passing the catalog name with a config file 
```python
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.
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!
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.
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.
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)
> Curious if there is an open issue discussing including this new feature.

I think only the discussions you are aware of, https://github.com/bodo-ai/Bodo-Pandas-Collaboration/issues/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.
I didn't know pytest fixtures could have tear down natively. Thanks for the feedback @mroeschke, I implement the Iceberg catalog as a fixture now, and also added the experimental warning as suggested. Please let me know if you have any other feedback, otherwise this should be ready.
> Could you also add a whatsnew entry in `v3.0.0.rst` under `Other enhancements`?

Absolutely, I thought I added it. Added it now