From b5f35a907eb39348757a0ac3acf28a8ad98a7318 Mon Sep 17 00:00:00 2001 From: TomNicholas Date: Sat, 21 Dec 2024 12:31:07 -0500 Subject: [PATCH 1/6] add test for FITS reader that checks if it can open Hubble data on AWS correctly --- virtualizarr/tests/test_readers/test_fits.py | 25 ++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 virtualizarr/tests/test_readers/test_fits.py diff --git a/virtualizarr/tests/test_readers/test_fits.py b/virtualizarr/tests/test_readers/test_fits.py new file mode 100644 index 00000000..15262d7e --- /dev/null +++ b/virtualizarr/tests/test_readers/test_fits.py @@ -0,0 +1,25 @@ +import pytest + +from xarray import Dataset + +from virtualizarr import open_virtual_dataset +from virtualizarr.tests import network, requires_kerchunk + + +pytest.importorskip("astropy") + + +@requires_kerchunk +@network +def test_open_hubble_data(): + # data from https://registry.opendata.aws/hst/ + vds = open_virtual_dataset( + "s3://stpubdata/hst/public/f05i/f05i0201m/f05i0201m_a1f.fits", + reader_options={'storage_options': {'anon': True}} + ) + + assert isinstance(vds, Dataset) + assert list(vds.variables) == ['PRIMARY'] + var = vds['PRIMARY'].variable + assert var.sizes == {'y': 17, 'x': 589} + assert var.dtype == '>i4' From 498565e2db1f5ed611b929304ba3828fb6070f80 Mon Sep 17 00:00:00 2001 From: TomNicholas Date: Sat, 21 Dec 2024 12:31:37 -0500 Subject: [PATCH 2/6] fix minor bug with fits reader arguments having to be specified explicitly --- virtualizarr/readers/fits.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/virtualizarr/readers/fits.py b/virtualizarr/readers/fits.py index 16ebe0ad..e9119ccb 100644 --- a/virtualizarr/readers/fits.py +++ b/virtualizarr/readers/fits.py @@ -42,7 +42,7 @@ def open_virtual_dataset( # TODO This wouldn't work until either you had an xarray backend for FITS installed, or issue #124 is implemented to load data from ManifestArrays directly # TODO Once we have one of those we can use ``maybe_open_loadable_vars_and_indexes`` here - if loadable_variables != [] or indexes != {} or decode_times: + if loadable_variables or indexes: raise NotImplementedError( "Cannot load variables or indexes from FITS files as there is no xarray backend engine for FITS" ) From ff64960d5f5a5e3e8e699d4418c6834972e417e2 Mon Sep 17 00:00:00 2001 From: TomNicholas Date: Sat, 21 Dec 2024 12:34:34 -0500 Subject: [PATCH 3/6] rename network -> requires_network to be more consistent with other test decorators --- virtualizarr/tests/__init__.py | 3 ++- virtualizarr/tests/test_backend.py | 6 +++--- virtualizarr/tests/test_readers/test_dmrpp.py | 4 ++-- virtualizarr/tests/test_readers/test_fits.py | 18 ++++++++---------- 4 files changed, 15 insertions(+), 16 deletions(-) diff --git a/virtualizarr/tests/__init__.py b/virtualizarr/tests/__init__.py index 658cf640..13fa2887 100644 --- a/virtualizarr/tests/__init__.py +++ b/virtualizarr/tests/__init__.py @@ -9,7 +9,8 @@ from virtualizarr.manifests.manifest import join from virtualizarr.zarr import ZArray, ceildiv -network = pytest.mark.network +# TODO rename to requires_network? +requires_network = pytest.mark.network def _importorskip( diff --git a/virtualizarr/tests/test_backend.py b/virtualizarr/tests/test_backend.py index 932fe7df..458cd1ae 100644 --- a/virtualizarr/tests/test_backend.py +++ b/virtualizarr/tests/test_backend.py @@ -15,8 +15,8 @@ from virtualizarr.readers.hdf import HDFVirtualBackend from virtualizarr.tests import ( has_astropy, - network, requires_kerchunk, + requires_network, requires_s3fs, requires_scipy, ) @@ -193,7 +193,7 @@ def test_var_attr_coords(self, netcdf4_file_with_2d_coords): assert set(vds.coords) == set(expected_coords) -@network +@requires_network @requires_s3fs class TestReadFromS3: @pytest.mark.parametrize( @@ -216,7 +216,7 @@ def test_anon_read_s3(self, indexes, hdf_backend): assert isinstance(vds[var].data, ManifestArray), var -@network +@requires_network @pytest.mark.parametrize("hdf_backend", [HDF5VirtualBackend, HDFVirtualBackend]) class TestReadFromURL: @pytest.mark.parametrize( diff --git a/virtualizarr/tests/test_readers/test_dmrpp.py b/virtualizarr/tests/test_readers/test_dmrpp.py index 4ef6b143..6bd8741a 100644 --- a/virtualizarr/tests/test_readers/test_dmrpp.py +++ b/virtualizarr/tests/test_readers/test_dmrpp.py @@ -11,7 +11,7 @@ from virtualizarr import open_virtual_dataset from virtualizarr.manifests.manifest import ChunkManifest from virtualizarr.readers.dmrpp import DMRParser -from virtualizarr.tests import network +from virtualizarr.tests import requires_network urls = [ ( @@ -177,7 +177,7 @@ def dmrparser(dmrpp_xml_str: str, tmp_path: Path, filename="test.nc") -> DMRPars ) -@network +@requires_network @pytest.mark.parametrize("data_url, dmrpp_url", urls) @pytest.mark.skip(reason="Fill_val mismatch") def test_NASA_dmrpp(data_url, dmrpp_url): diff --git a/virtualizarr/tests/test_readers/test_fits.py b/virtualizarr/tests/test_readers/test_fits.py index 15262d7e..cdaeeab8 100644 --- a/virtualizarr/tests/test_readers/test_fits.py +++ b/virtualizarr/tests/test_readers/test_fits.py @@ -1,25 +1,23 @@ import pytest - from xarray import Dataset from virtualizarr import open_virtual_dataset -from virtualizarr.tests import network, requires_kerchunk - +from virtualizarr.tests import requires_kerchunk, requires_network pytest.importorskip("astropy") @requires_kerchunk -@network +@requires_network def test_open_hubble_data(): # data from https://registry.opendata.aws/hst/ vds = open_virtual_dataset( "s3://stpubdata/hst/public/f05i/f05i0201m/f05i0201m_a1f.fits", - reader_options={'storage_options': {'anon': True}} + reader_options={"storage_options": {"anon": True}}, ) - + assert isinstance(vds, Dataset) - assert list(vds.variables) == ['PRIMARY'] - var = vds['PRIMARY'].variable - assert var.sizes == {'y': 17, 'x': 589} - assert var.dtype == '>i4' + assert list(vds.variables) == ["PRIMARY"] + var = vds["PRIMARY"].variable + assert var.sizes == {"y": 17, "x": 589} + assert var.dtype == ">i4" From 85587f4e2a9a84f2907c9966e74900799677062d Mon Sep 17 00:00:00 2001 From: TomNicholas Date: Sat, 21 Dec 2024 12:35:58 -0500 Subject: [PATCH 4/6] remove ToDO --- virtualizarr/tests/__init__.py | 1 - 1 file changed, 1 deletion(-) diff --git a/virtualizarr/tests/__init__.py b/virtualizarr/tests/__init__.py index 13fa2887..b1fbdf11 100644 --- a/virtualizarr/tests/__init__.py +++ b/virtualizarr/tests/__init__.py @@ -9,7 +9,6 @@ from virtualizarr.manifests.manifest import join from virtualizarr.zarr import ZArray, ceildiv -# TODO rename to requires_network? requires_network = pytest.mark.network From 8feb780d6ebb7fb39bb40288b931b444307a59f7 Mon Sep 17 00:00:00 2001 From: TomNicholas Date: Sat, 21 Dec 2024 12:43:52 -0500 Subject: [PATCH 5/6] release note --- docs/releases.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/releases.rst b/docs/releases.rst index 4146948f..e4829d13 100644 --- a/docs/releases.rst +++ b/docs/releases.rst @@ -31,6 +31,8 @@ Bug fixes - Fix bug preventing generating references for the root group of a file when a subgroup exists. (:issue:`336`, :pull:`338`) By `Tom Nicholas `_. +- Fix bug passing arguments to FITS reader, and test it on Hubble Space Telescope data. + (:pull:`363`) By `Tom Nicholas `_. Documentation ~~~~~~~~~~~~~ From 7742a2f4da71c21180a7af6ae5f647c90fbaee4d Mon Sep 17 00:00:00 2001 From: TomNicholas Date: Sat, 21 Dec 2024 20:27:19 -0500 Subject: [PATCH 6/6] re-trigger CI