fix: Cache name resolver improvements addressing Copilot PR review

- Use instance-level getattr for proper property descriptor resolution
- Add type validation to ensure string returns
- Update docstrings from 'should' to 'may' to reflect optional overrides
- Add regression tests for MRO edge case handling

Author: kunal-10-cloud

malariagen_data/anopheles.py

@@ -182,26 +182,44 @@ def __init__(
     def _get_xpehh_gwss_cache_name(self):
         """Safe resolver for xpehh gwss cache name.
 
-        Subclasses should define _xpehh_gwss_cache_name as a class attribute.
+        Subclasses may define _xpehh_gwss_cache_name as a class attribute.
         This method safely retrieves it or returns a default if unavailable.
         """
-        cache_name = getattr(self.__class__, "_xpehh_gwss_cache_name", None)
-        if cache_name is None:
+        try:
+            # Try to get the cache name from the instance (respects MRO)
+            cache_name = getattr(self, "_xpehh_gwss_cache_name")
+            # Validate it's actually a string, not a descriptor or other object
+            if not isinstance(cache_name, str):
+                raise TypeError(
+                    f"_xpehh_gwss_cache_name must resolve to a string, "
+                    f"got {type(cache_name).__name__}"
+                )
+            return cache_name
+        except (AttributeError, TypeError):
             # Fallback to a generic cache name if subclass hasn't defined one
-            cache_name = "xpehh_gwss_v1"
-        return cache_name
+            # or if the attribute isn't a proper string
+            return "xpehh_gwss_v1"
 
     def _get_ihs_gwss_cache_name(self):
         """Safe resolver for ihs gwss cache name.
 
-        Subclasses should define _ihs_gwss_cache_name as a class attribute.
+        Subclasses may define _ihs_gwss_cache_name as a class attribute.
         This method safely retrieves it or returns a default if unavailable.
         """
-        cache_name = getattr(self.__class__, "_ihs_gwss_cache_name", None)
-        if cache_name is None:
+        try:
+            # Try to get the cache name from the instance (respects MRO)
+            cache_name = getattr(self, "_ihs_gwss_cache_name")
+            # Validate it's actually a string, not a descriptor or other object
+            if not isinstance(cache_name, str):
+                raise TypeError(
+                    f"_ihs_gwss_cache_name must resolve to a string, "
+                    f"got {type(cache_name).__name__}"
+                )
+            return cache_name
+        except (AttributeError, TypeError):
             # Fallback to a generic cache name if subclass hasn't defined one
-            cache_name = "ihs_gwss_v1"
-        return cache_name
+            # or if the attribute isn't a proper string
+            return "ihs_gwss_v1"
 
     @staticmethod
     def _make_gene_cnv_label(gene_id, gene_name, cnv_type):

tests/anoph/test_cache_name_resolvers.py

@@ -0,0 +1,50 @@
+"""Test cache name resolver methods for MRO edge case handling."""
+
+from malariagen_data.anopheles import AnophelesDataResource
+
+
+class DummyAnophelesNoOverrides(AnophelesDataResource):
+    """Dummy subclass that doesn't override cache name properties.
+
+    Tests the edge case where resolver methods must return defaults
+    instead of raising NotImplementedError.
+    """
+
+    def __init__(self):
+        # Minimal initialization - we only test the resolver methods
+        pass
+
+
+def test_get_xpehh_gwss_cache_name_returns_default():
+    """Test that _get_xpehh_gwss_cache_name returns default when not overridden."""
+    instance = DummyAnophelesNoOverrides()
+
+    # Should not raise NotImplementedError
+    cache_name = instance._get_xpehh_gwss_cache_name()
+
+    # Should return default string
+    assert isinstance(cache_name, str)
+    assert cache_name == "xpehh_gwss_v1"
+
+
+def test_get_ihs_gwss_cache_name_returns_default():
+    """Test that _get_ihs_gwss_cache_name returns default when not overridden."""
+    instance = DummyAnophelesNoOverrides()
+
+    # Should not raise NotImplementedError
+    cache_name = instance._get_ihs_gwss_cache_name()
+
+    # Should return default string
+    assert isinstance(cache_name, str)
+    assert cache_name == "ihs_gwss_v1"
+
+
+def test_cache_name_resolvers_always_return_string():
+    """Test that both resolvers always return strings, never other types."""
+    instance = DummyAnophelesNoOverrides()
+
+    xpehh_name = instance._get_xpehh_gwss_cache_name()
+    ihs_name = instance._get_ihs_gwss_cache_name()
+
+    assert isinstance(xpehh_name, str), f"Expected str, got {type(xpehh_name).__name__}"
+    assert isinstance(ihs_name, str), f"Expected str, got {type(ihs_name).__name__}"