pydata / xarray

N-D labeled arrays and datasets in Python
https://xarray.dev
Apache License 2.0
3.63k stars 1.09k forks source link

Fix open_mfdataset for list of fsspec files #9785

Closed phofl closed 6 days ago

phofl commented 6 days ago

cc @dcherian

dcherian commented 6 days ago

mypy needs a fix. Can you expand that logic out to an explicit for-loop please. The comprehension is quite hard to read now.

phofl commented 6 days ago

I added type ignores, mypy should have raised earlier as well since the else clause was incorrect, but type inference was probably a bit smarter here. I really don't know enough about mypy to properly troubleshoot this

phofl commented 6 days ago

The type hints are generally incorrect in open_mfdataset (I think) since the fsspec version doesn't produce a string or os.PathLike

headtr1ck commented 6 days ago

The type hints are generally incorrect in open_mfdataset (I think) since the fsspec version doesn't produce a string or os.PathLike

Do you know what this returns? Is it some buffered reader or similar?

phofl commented 6 days ago

No, no clue. Nothing is typed over there and there are so many subclasses out there...

The class AbstractFileSystem implements the open but again not really clear

We typed this as

ReadBuffer[bytes]

in pandas, but this is very generic. The implementation lives here: https://github.com/pandas-dev/pandas/blob/ee3c18f51b393893ed6e31214c7be2f9427ce0c9/pandas/_typing.py#L270

dcherian commented 6 days ago

OK merging since this fixes a regression. We can followup with typing improvements.