Closed TomAugspurger closed 4 months ago
Hey @TomAugspurger, I'm not getting that failure when running the tests on your branch, which is probably due to some AWS credential magic on my end. Updating the tests to run against minio or something similar would probably help with that..
Thanks for the PR @TomAugspurger.
I just tried this out and am running into some unexpected behavior:
I installed virtualizarr from this PR branc
!pip install git+https://github.com/TomAugspurger/VirtualiZarr.git@user/tom/feature/filesystems
For the setup
from virtualizarr import open_virtual_dataset
from virtualizarr.kerchunk import FileType
urls = [
"http://aims3.llnl.gov/thredds/fileServer/css03_data/CMIP6/ScenarioMIP/DKRZ/MPI-ESM1-2-HR/ssp126/r1i1p1f1/Amon/tas/gn/v20190710/tas_Amon_MPI-ESM1-2-HR_ssp126_r1i1p1f1_gn_201501-201912.nc",
"http://aims3.llnl.gov/thredds/fileServer/css03_data/CMIP6/ScenarioMIP/DKRZ/MPI-ESM1-2-HR/ssp126/r1i1p1f1/Amon/tas/gn/v20190710/tas_Amon_MPI-ESM1-2-HR_ssp126_r1i1p1f1_gn_202001-202412.nc",
"http://aims3.llnl.gov/thredds/fileServer/css03_data/CMIP6/ScenarioMIP/DKRZ/MPI-ESM1-2-HR/ssp126/r1i1p1f1/Amon/tas/gn/v20190710/tas_Amon_MPI-ESM1-2-HR_ssp126_r1i1p1f1_gn_202501-202912.nc",
"http://aims3.llnl.gov/thredds/fileServer/css03_data/CMIP6/ScenarioMIP/DKRZ/MPI-ESM1-2-HR/ssp126/r1i1p1f1/Amon/tas/gn/v20190710/tas_Amon_MPI-ESM1-2-HR_ssp126_r1i1p1f1_gn_203001-203412.nc",
]
Then I tried to naively do this:
vds_list = []
for url in urls:
vds = open_virtual_dataset(
url, indexes={}
)
vds_list.append(vds)
which failed with this error
TypeError: ClientSession._request() got an unexpected keyword argument 'key'
Makes me think that the protocol is not properly detected?
When I add reader_options={}
it works as intended:
vds_list = []
for url in tqdm(urls):
vds = open_virtual_dataset(
url, indexes={}, reader_options={}
)
vds_list.append(vds)
I believe that the default values for reader options are basically invalidating this logic.
Thanks for this contribution @TomAugspurger !
If @norlandrhagen is happy with this (including @jbusecke 's fix), then I am happy to merge it.
100% Thanks for the fixes @TomAugspurger and @jbusecke!
Oh weird. My test case is still failing after pulling from main? My fix might have not been sufficient?
Damn - is there a test/reproducer for this issue @jbusecke (that you can raise in a new issue)?
Yeah I have that on my list, but very busy this week, so might have to push to next week. Please ping me as needed 😆
@TomNicholas see https://github.com/zarr-developers/VirtualiZarr/issues/135
This change simplifies the handling of filepaths in
_fsspec_openfile_from_filepath
, and removes some restrictions around what can be passed in. Most notably, it allows the use of non-S3 and local filepaths.I see that
virtualizarr/tests/test_xarray.py::test_anon_read_s3
covers this, but that's failing for me onmain
withIs that just a local configuration issue for me, or is it failing for others as well?