From d4f6b98ac3cce823d7fa57956e824a93effa6e94 Mon Sep 17 00:00:00 2001
From: Tom Nicholas <tom@cworthy.org>
Date: Thu, 9 Jan 2025 08:08:28 -0700
Subject: [PATCH] Test FITS reader on Hubble Space Telescope data (#363)

* add test for FITS reader that checks if it can open Hubble data on AWS correctly

* fix minor bug with fits reader arguments having to be specified explicitly

* rename network -> requires_network to be more consistent with other test decorators

* remove ToDO

* release note

* re-trigger CI
---
 docs/releases.rst                             |  2 ++
 virtualizarr/readers/fits.py                  |  2 +-
 virtualizarr/tests/__init__.py                |  2 +-
 virtualizarr/tests/test_backend.py            |  6 ++---
 virtualizarr/tests/test_readers/test_dmrpp.py |  4 ++--
 virtualizarr/tests/test_readers/test_fits.py  | 23 +++++++++++++++++++
 6 files changed, 32 insertions(+), 7 deletions(-)
 create mode 100644 virtualizarr/tests/test_readers/test_fits.py

diff --git a/docs/releases.rst b/docs/releases.rst
index 7b2df3ed..6b2bfd92 100644
--- a/docs/releases.rst
+++ b/docs/releases.rst
@@ -35,6 +35,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 <https://github.com/TomNicholas>`_.
+- Fix bug passing arguments to FITS reader, and test it on Hubble Space Telescope data.
+  (:pull:`363`) By `Tom Nicholas <https://github.com/TomNicholas>`_.
 
 Documentation
 ~~~~~~~~~~~~~
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"
             )
diff --git a/virtualizarr/tests/__init__.py b/virtualizarr/tests/__init__.py
index 658cf640..b1fbdf11 100644
--- a/virtualizarr/tests/__init__.py
+++ b/virtualizarr/tests/__init__.py
@@ -9,7 +9,7 @@
 from virtualizarr.manifests.manifest import join
 from virtualizarr.zarr import ZArray, ceildiv
 
-network = pytest.mark.network
+requires_network = pytest.mark.network
 
 
 def _importorskip(
diff --git a/virtualizarr/tests/test_backend.py b/virtualizarr/tests/test_backend.py
index c9347786..414fa571 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
new file mode 100644
index 00000000..cdaeeab8
--- /dev/null
+++ b/virtualizarr/tests/test_readers/test_fits.py
@@ -0,0 +1,23 @@
+import pytest
+from xarray import Dataset
+
+from virtualizarr import open_virtual_dataset
+from virtualizarr.tests import requires_kerchunk, requires_network
+
+pytest.importorskip("astropy")
+
+
+@requires_kerchunk
+@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}},
+    )
+
+    assert isinstance(vds, Dataset)
+    assert list(vds.variables) == ["PRIMARY"]
+    var = vds["PRIMARY"].variable
+    assert var.sizes == {"y": 17, "x": 589}
+    assert var.dtype == ">i4"