method resolution surprise on EarthAccessFile

[itcarroll]

There's something funny going on with the wrapping of fsspec.spec.AbstractBufferedFile, where the method resolution isn't coming out as expected.

The following gives me an AssertionError

import earthaccess

auth = earthaccess.login()
results = earthaccess.search_data(short_name="VIIRSJ1_L2_OC", count=1)
files = earthaccess.open(results)
with files[0] as g:
    assert g.read is getattr(g.f, "read")

I don't know about the internals, but getattr(g, "read") and g.__getattribute__("read") both follow the MRO to the read method of fsspec.spec.AbstractBufferedFile. We don't want that! We want the read method, given by g.__getattr__("read") as fsspec.implementations.http.HTTPFile.read as resulting from getattr(g.f) in the class definition.

earthaccess/earthaccess/store.py

Lines 28 to 34 in b7443fe

	class EarthAccessFile(fsspec.spec.AbstractBufferedFile):
	def __init__(self, f: fsspec.AbstractFileSystem, granule: DataGranule) -> None:
	self.f = f
	self.granule = granule
	def __getattr__(self, method: str) -> Any:
	return getattr(self.f, method)

Is it possible that __getattr__ should be __getattribute__?

Sorry for the multiple edits; confused myself with two fs

[itcarroll]

No, don't use __getattribute__ b/c it leads to infinite recursion. New idea: this is not a class that should be subclassing at all.

[itcarroll]

When #620 removed the inherited fsspec.spec.AbstractBufferedFile class from earthaccess.EarthAccessFile, it broke the ability of xarray.backends.list_engines()["h5netcdf"].guess_can_open to recognize these pointers. It comes back to MRO, and the fact that EarthAccessFile is no longer an instance of io.IOBase.

[itcarroll]

It would be very helpful if we had an integration test for the behavior that the EarthAccessFile class enables.

[itcarroll]

A change to repair damage done in #620 is to reintroduce a base class on EarthAccessFile (and I think io.IOBase might work), but it doesn't fix this issue.

[itcarroll]

Merged #832 in order to make way for a release. Note that the test surfacing this bug has been marked as an expected fail.

earthaccess/tests/unit/test_store.py

Lines 132 to 141 in fb78806

	@pytest.mark.xfail(
	reason="This test reproduces a bug (#610) which has not yet been fixed."
	)
	def test_earthaccess_file_getattr():
	fs = fsspec.filesystem("memory")
	with fs.open("/foo", "wb") as f:
	earthaccess_file = EarthAccessFile(f, granule="foo")
	assert f.tell() == earthaccess_file.tell()
	# cleanup
	fs.store.clear()