Fix issue with encoding of arrays/list when cache is created

[agrouaze]

The purpose of this PR is to fix #135

After correction the cache is correctly created and re-use it is working successfully:

du -sh ~/.cache/xarray-ceos-alos2/b5e1d732947425674756589dd1aa0e7f6350ec6200eaaabaecbb4f9a397752d4/IM*
65M	.cache/xarray-ceos-alos2/b5e1d732947425674756589dd1aa0e7f6350ec6200eaaabaecbb4f9a397752d4/IMG-HH-ALOS2281963100-190813-WWDR1.1__D-B1.index
81M	.cache/xarray-ceos-alos2/b5e1d732947425674756589dd1aa0e7f6350ec6200eaaabaecbb4f9a397752d4/IMG-HH-ALOS2281963100-190813-WWDR1.1__D-B2.index
65M	.cache/xarray-ceos-alos2/b5e1d732947425674756589dd1aa0e7f6350ec6200eaaabaecbb4f9a397752d4/IMG-HH-ALOS2281963100-190813-WWDR1.1__D-B3.index
65M	.cache/xarray-ceos-alos2/b5e1d732947425674756589dd1aa0e7f6350ec6200eaaabaecbb4f9a397752d4/IMG-HH-ALOS2281963100-190813-WWDR1.1__D-B4.index
...
81M	.cache/xarray-ceos-alos2/b5e1d732947425674756589dd1aa0e7f6350ec6200eaaabaecbb4f9a397752d4/IMG-HV-ALOS2281963100-190813-WWDR1.1__D-B5.index

[keewis]

this can be simplified by extending Variable.__post_init__ with:

if isinstance(self.data, list):
    super().__setattr__("data", np.array(self.data))

Even better would be to do this immediately after parsing because that would help us use the byte structure to determine a dtype, and thus avoid platform-specific behavior (e.g. windows will choose 32-bit dtypes instead of 64-bit dtypes by default). I haven't had time to look into what it would take to do that, but in the meantime we could do

if isinstance(self.data, list):
    super().__setattr__("data", np.array(self.data, dtype=infer_dtype(self.data)))

and then define infer_dtype as something like

def infer_dtype(x):
    match x:
        case np.ndarray() | np.generic():
            return x.dtype
        case list([int()]):
            return np.int64
        case list([float()]):
           return np.float64
        case list([str()]):
            return np.dtypes.StringDType
        case _:
            return None

[agrouaze]

I implemented something very close to what you propose and it is working.
let me know if you prefer to for for the match / case syntax.

[keewis]

I don't have a strong attachment to the match / case syntax, but I do believe that the pattern matching is more readable than the chain of comparisons

[keewis]

I don't have a strong attachment to the match / case syntax, but I do believe that the pattern matching is more readable than the chain of comparisons

[keewis]

might need a test to avoid regressing, though

[agrouaze]

what you suggested was (in test_hierarchy.py)

class TestVariable:
    @pytest.mark.parametrize(
        ["dims", "data"],
        (
            pytest.param("x", np.arange(5), id="str-1d"),
            pytest.param(["x"], [1, 2, 3, 4, 5], id="list-1d-pure"),
            pytest.param(["x"], np.arange(5), id="list-1d"),
            pytest.param(["x", "y"], np.arange(6).reshape(3, 2), id="list-2d"),
        ),
    )

?
because if so, it crashes:

...
ceos_alos2/tests/test_hierarchy.py:34: AssertionError
______________________________________________________________________________________________ TestVariable.test_init[attrs1-list-1d-pure] ______________________________________________________________________________________________

self = <ceos_alos2.tests.test_hierarchy.TestVariable object at 0x7f7f44fdff00>, dims = ['x'], data = [1, 2, 3, 4, 5], attrs = {'a': 1}

    @pytest.mark.parametrize(
        ["dims", "data"],
        (
            pytest.param("x", np.arange(5), id="str-1d"),
            pytest.param(["x"], [1, 2, 3, 4, 5], id="list-1d-pure"),
            pytest.param(["x"], np.arange(5), id="list-1d"),
            pytest.param(["x", "y"], np.arange(6).reshape(3, 2), id="list-2d"),
        ),
    )
    @pytest.mark.parametrize(
        "attrs",
        (
            {},
            {"a": 1},
        ),
    )
    def test_init(self, dims, data, attrs):
        var = Variable(dims=dims, data=data, attrs=attrs)
    
        if isinstance(dims, str):
            dims = [dims]
    
        assert var.dims == dims
>       assert type(var.data) is type(data) and np.all(var.data == data)
E       AssertionError: assert (<class 'numpy.ndarray'> is <class 'list'>)
E        +  where <class 'numpy.ndarray'> = type(array([1, 2, 3, 4, 5]))
E        +    where array([1, 2, 3, 4, 5]) = Variable(dims=['x'], data=array([1, 2, 3, 4, 5]), attrs={'a': 1}).data
E        +  and   <class 'list'> = type([1, 2, 3, 4, 5])

ceos_alos2/tests/test_hierarchy.py:34: AssertionError
=========================================================================================================== warnings summary ============================================================================================================
ceos_alos2/tests/test_summary.py::test_categorize_filenames[short]
  /scale/user/agrouaze/micromamba/envs/dev3.14/lib/python3.14/site-packages/_pytest/raises.py:622: PytestWarning: matching against an empty string will *always* pass. If you want to check for an empty message you need to pass '^$'. If you don't want to match you should pass `None` or leave out the parameter.
    super().__init__(match=match, check=check)

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
======================================================================================================== short test summary info ========================================================================================================
FAILED ceos_alos2/tests/test_hierarchy.py::TestVariable::test_init[attrs0-list-1d-pure] - AssertionError: assert (<class 'numpy.ndarray'> is <class 'list'>)
FAILED ceos_alos2/tests/test_hierarchy.py::TestVariable::test_init[attrs1-list-1d-pure] - AssertionError: assert (<class 'numpy.ndarray'> is <class 'list'>)
========================================================================================= 2 failed, 1223 passed, 1 skipped, 1 warning in 4.95s ====

might need a test to avoid regressing, though

[keewis]

well, that's something you can easily change: the test checks type(var.data) is type(data), which only works if you always pass numpy.ndarray objects. There's nothing that keeps you from changing that to type(var.data) is np.ndarray or even isinstance(var.data, np.ndarray), which gets the test to pass for me locally.

Edit: but yes, that was what I was proposing.