From f2cc19de9098cb8987c09332eb55a8297723c3fc Mon Sep 17 00:00:00 2001
From: Bryna Hazelton <bryna.hazelton@gmail.com>
Date: Fri, 9 Feb 2024 15:09:21 -0800
Subject: [PATCH] Add test for unknown catalog name in FHD file

Improve error message, update changelog
---
 CHANGELOG.md                            |   1 +
 src/pyradiosky/data/fhd_catalog_bad.sav | Bin 0 -> 3068 bytes
 src/pyradiosky/skymodel.py              |   5 +++-
 tests/test_skymodel.py                  |  37 ++++++++++++++++--------
 4 files changed, 30 insertions(+), 13 deletions(-)
 create mode 100644 src/pyradiosky/data/fhd_catalog_bad.sav

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 6e6b538a..1c25c8ce 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -6,6 +6,7 @@
 - A memo describing the SkyH5 format in docs/references.
 
 ### Changed
+- Added support for more types of FHD catalog files and better error messages.
 - Added support (and testing) for python 3.12, dropped support for python 3.8
 - Moved the optional data-sized arrays (stokes_error and beam_amp) from the Header
 datagroup to the Data datagroup in SkyH5 files.
diff --git a/src/pyradiosky/data/fhd_catalog_bad.sav b/src/pyradiosky/data/fhd_catalog_bad.sav
new file mode 100644
index 0000000000000000000000000000000000000000..b871fde94971548ab39969ea98a67116141ab184
GIT binary patch
literal 3068
zcmWFzVqjrlVBlh4U`YVdqhK@yMnhmU1SH&wG8NoXlN1yz6%0+ROpUEfj1-Ivj7)$*
z!<tl7nU@F&Be~+loXiScXEQ?sy@HI&V!iUj;*89^^pgBMz0{Oaup&NS*)jtuNO^#4
zwh9Zgcr%b`Kz2%EQF&$_n9XXTXQ5{ZW->4_FmS(tFu^2yaFBDnV^9!?!vVw~-5|Py
zfq~foNV5SkBM`#?NDd|j632!)p^8Cjf$oF@W+)3(Hi9q+fYiX)AV0v^0!VBTB(?++
zTLy`(fW%e-vVr~t0}UVvEQi5B2S|eK1NkuoiJgGNX7Y3a@__z|08<Q%kr0|G$Pvt7
zc5!tEF<62_{QW?*TaarYkY;uC3Gjd@VPJ80^n}RyIJyJn*jyt*T>T*Cvbgz#f{bPj
z_HYbv1u}qu#mUvt7f6HL268V5!@?EF1Ot#9ObizdQUeM@5QeD%GQogY{UEzRm>m6}
zc!&89WGTo_7=Xnqh=#F2@}RT?0-*SYv5Adim^n-lkjP?+ghUoo1eA`1h%<VEtzcjT
zMHCP)hC=Bu2#uUxVQCa312Z4p%`kEFG{WlW=k5cIQ*c;7;}vAqPzeimkYinVHs`vC
z#=1ChZSOvK%=@0R8p|VRm2YMHea@V+Zvd%)0DI|kYwQya3B&j>+G+p2DKIuVZMWXm
z&hCH@C{Ggtj4s?QJkHnWy>;RiK6prQgR|2%p^rd!Q~=#^7R4RRNgwQWW%<z!bg~i-
zM;C+f?M~ZD+THju8OFy$Gr0&jRJ(92F>~VI`QhNB%F|9uKU{#i<2;Hxu3lMUzi7n`
zbh8}4pHfB_gYoT!{_e5oIteMMVS?CcW*4RzUC!rsOmqBOmUKug($6V`%?j#{bI9(n
zGkWA`w>PQ_-5AGjv$D~}V0^nJ-zw}*buWkU@z5+TB7%}G`~n-DM6^#Gyu8}fNoH9E
zws^7Az1VE~M??YLEXRGDR-%i+_;%Y5d)lpd_!h>;L$kUFOw)9}zE#IbNJ03JU<#9y
xwE174JIaCS10_6cA7<CuepSUtr;aBk7UDABZb_r6-SYpxaLFTy!^$nN5CEk7#uoqp

literal 0
HcmV?d00001

diff --git a/src/pyradiosky/skymodel.py b/src/pyradiosky/skymodel.py
index 818805bc..c4e59380 100644
--- a/src/pyradiosky/skymodel.py
+++ b/src/pyradiosky/skymodel.py
@@ -4357,7 +4357,10 @@ def read_fhd_catalog(
         elif "source_array" in catalog.keys():
             catalog = catalog["source_array"]
         else:
-            raise KeyError(f"File {filename_sav} does not contain a known catalog name")
+            raise KeyError(
+                f"File {filename_sav} does not contain a known catalog name. "
+                f"File variables include {list(catalog.keys())}"
+            )
         ids = catalog["id"].astype(str)
         ra = catalog["ra"]
         dec = catalog["dec"]
diff --git a/tests/test_skymodel.py b/tests/test_skymodel.py
index 6704fff2..ce55bd9c 100644
--- a/tests/test_skymodel.py
+++ b/tests/test_skymodel.py
@@ -2941,21 +2941,21 @@ def test_read_votable_errors():
         )
 
 
-def test_fhd_catalog_reader():
-    for fname in ["catalog", "source_array"]:
-        catfile = os.path.join(SKY_DATA_PATH, f"fhd_{fname}.sav")
+@pytest.mark.parametrize("fname", ["catalog", "source_array"])
+def test_fhd_catalog_reader(fname):
+    catfile = os.path.join(SKY_DATA_PATH, f"fhd_{fname}.sav")
 
-        if fname == "catalog":
-            with uvtest.check_warnings(
-                UserWarning, match="Source IDs are not unique. Defining unique IDs."
-            ):
-                skyobj = SkyModel.from_fhd_catalog(catfile, expand_extended=False)
-        else:
+    if fname == "catalog":
+        with uvtest.check_warnings(
+            UserWarning, match="Source IDs are not unique. Defining unique IDs."
+        ):
             skyobj = SkyModel.from_fhd_catalog(catfile, expand_extended=False)
+    else:
+        skyobj = SkyModel.from_fhd_catalog(catfile, expand_extended=False)
 
-        assert skyobj.filename == [f"fhd_{fname}.sav"]
-        catalog = scipy.io.readsav(catfile)[fname]
-        assert skyobj.Ncomponents == len(catalog)
+    assert skyobj.filename == [f"fhd_{fname}.sav"]
+    catalog = scipy.io.readsav(catfile)[fname]
+    assert skyobj.Ncomponents == len(catalog)
 
     assert np.all(skyobj.reference_frequency > 50 * units.MHz)
 
@@ -3042,6 +3042,19 @@ def test_fhd_catalog_reader_labeling_extended_sources():
         assert skyobj.name[comp] == expected_name[comp]
 
 
+def test_fhd_catalog_reader_errors():
+    catfile = os.path.join(SKY_DATA_PATH, "fhd_catalog_bad.sav")
+
+    with pytest.raises(
+        KeyError,
+        match=re.escape(
+            f"File {catfile} does not contain a known catalog name. "
+            "File variables include ['src_arr']"
+        ),
+    ):
+        SkyModel.from_fhd_catalog(catfile)
+
+
 def test_point_catalog_reader():
     catfile = os.path.join(SKY_DATA_PATH, "pointsource_catalog.txt")
     skyobj = SkyModel.from_file(catfile)