nsidc
`download` returns inconsistent types (`Path`s or `str`s) depending on `in_region`
The download docs say that the return value shall be a list of strings. When running in_region however, the result is a list of pathlib.PosixPath objects.
import earthaccess
import pathlib
assert earthaccess.__version__ == "0.9.0"
assert earthaccess.__store__.in_region
local = pathlib.Path("data").mkdir(exist_ok=True)
results = earthaccess.search_data(short_name="PACE_OCI_L2_BGC_NRT", count=1)
paths = earthaccess.download(results, local_path=local)
assert isinstance(paths[0], str)
The last assert raises an exception.
Nice catch! Why isn't our type checker catching this though?
https://github.com/nsidc/earthaccess/blob/9f43047f6d2f6845d33a9367814262b43d9bcc46/earthaccess/api.py#L177
I love pathlib though, should we update download() to always return Paths? :)
If you want to be fancy, you could return the same type given to you in the local_path argument.
I like the idea of returning a list of Path objects. I think this would avoid cross platform issues.
I would like to upvote this for earthaccess.download() to be consistently returning an object of the same type, whether it be Posix or string (probably should be Posix for cross platform compatibility although several folks may have developed code already assuming the string list). In my script, I evaluate the earthaccess.download() returned object within my script to check whether I am in region or not and then apply list comprehension line to convert the Posix object to a string list since another function is expecting a string list.
files = earthaccess.download(granules, path)
if earthaccess.__store__.in_region == True: files = [str(Path(f)) for f in files]
although several folks may have developed code already assuming the string list
Excellent point. Let's please keep in mind as we address this that we also have work in progress to establish a compatibility policy: https://github.com/nsidc/earthaccess/pull/763/files We should include a prominent notification of breaking change and a migration guide with code examples.
@mfisher87 I want to work on this one. Should we proceed forward with the suggestion above to return Posix whether in region or out of region?
Yes, I think we can safely decide to return a Path (not a PosixPath, some users are on Windows!). We already switched to returning Path in some cases, and I think that was driven by a general preference for Paths over strs. Thanks, Sherwin!
