[BUG] incorrect typing on get_s3_filesystem has led open_virtual_mfdataset astray

[itcarroll]

Is this issue already tracked somewhere, or is this a new report?

• I've reviewed existing issues and couldn't find a duplicate for this problem.

Current Behavior

The earthaccess.open_virtual_mfdataset function with access="direct" returns a KeyError: 0.

Here the earthaccess.get_s3_filesystem is given granules[0] when it should be given granules.

earthaccess/earthaccess/dmrpp_zarr.py

Line 95 in b9dec8b

fs = earthaccess.get_s3_filesystem(results=granules[0])

Here the typing is misleading and should be Optional[list[DataGranule]].

earthaccess/earthaccess/api.py

Line 402 in b9dec8b

results: Optional[DataGranule] = None,

Expected Behavior

The earthaccess.open_virtual_mfdataset function with access="direct" returns a dataset.

Steps To Reproduce

import earthaccess

results = earthaccess.search_data(
    short_name="MUR-JPL-L4-GLOB-v4.1",
    temporal=("2010-01-01", "2010-01-31"),
)
earthaccess.open_virtual_mfdataset(
    results,
    access="direct",
    load=False,
    concat_dim="time",
    coords="all",
    compat="override",
    combine_attrs="drop_conflicts",
)

Environment

- OS: openscapes.2i2c
- Python: 3.12.0

Additional Context

No response

[ayushnag]

@betolink This should be a pretty quick fix. Can I also change this line of get_s3_filesystem to just use a single granule result rather than results[0]? Or should the type hint be modified to accept Optional[list[DataGranule]]?

[itcarroll]

I suspect if you change the signature of get_s3_filesystem (results -> result) there would be much breakage among calls that pass a list. I think you want to correct the typing. If that breaks the mypy checks though, then I don't know!

Oh, and it looks like we use List from typing rather than list. Just noticed that.

[betolink]

@ayushnag Agreed, I think using a list is still the recommended way to go and this is an artifact of some missions having their own endpoints like SWOT and NISAR etc. @itcarroll is there a difference in List vs list? which one should we use in this case?

[itcarroll]

typing.List is a deprecated alias to list. Matter of preference, so I guess I would just be consistent within any given module.

[danielfromearth]

I am running into a KeyError too when using access='direct' (see screenshot).

It seems like it's probably the same as what's already in this issue, but I'm not totally sure... can someone check that it looks the same and that no new issue is necessary?

[mfisher87]

is there a difference in List vs list? which one should we use in this case?

We should use Ruff to upgrade our whole codebase to use builtins now that 3.9 is our oldest supported version. It's in the pyupgrade ruleset (UP) if I remember correctly!

[ayushnag]

It seems like it's probably the same as what's already in this issue, but I'm not totally sure... can someone check that it looks the same and that no new issue is necessary?

Yes, this is the same issue. There is a PR to fix it that should be merged soon (#923)