pysal / libpysal

Core components of Python Spatial Analysis Library
http://pysal.org/libpysal
Other
266 stars 78 forks source link

`libpysal/weights/raster.py` missing test coverage #657

Open jGaboardi opened 1 year ago

jGaboardi commented 1 year ago

xref https://github.com/pysal/libpysal/pull/655#pullrequestreview-1733827078

Although pytest claims that this chunk is being tested in libpysal/weights/raster.py, it is in fact not. As an example:

from libpysal.weights import raster
pytest.importorskip("xarray")
da2_missing_in = raster.testDataArray((1, 4, 4), missing_vals=True)
w2_missing = raster.da2W(da2_missing_in, "rook", n_jobs=-1)
da2_missing_out = raster.w2da(
    da2_missing_in.data.flatten(), w2_missing, da2_missing_in.attrs, None
)
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
Cell In[2], line 6
      4 da2_missing_in = raster.testDataArray((1, 4, 4), missing_vals=True)
      5 w2_missing = raster.da2W(da2_missing_in, "rook", n_jobs=-1)
----> 6 da2_missing_out = raster.w2da(
      7     da2_missing_in.data.flatten(), w2_missing, da2_missing_in.attrs, None
      8 )

File ~/libpysal/libpysal/weights/raster.py:343, in w2da(data, w, attrs, coords)
    341     raise TypeError("w must be an instance of weights.W")
    342 if hasattr(w, "index"):
--> 343     da = _index2da(data, w.index, attrs, coords)
    344 else:
    345     raise AttributeError(
    346         "This method requires `w` object to include `index` "
    347         "attribute that is built as a `pandas.MultiIndex` object."
    348     )

File ~/libpysal/libpysal/weights/raster.py:570, in _index2da(data, index, attrs, coords)
    568 else:
    569     data_complete = np.empty(shape, data.dtype)
--> 570 data_complete[indexer] = data
    571 coords = {}
    572 for dim, lev in zip(dims, idx.levels, strict=True):

ValueError: shape mismatch: value array of shape (16,) could not be broadcast to indexing result of shape (6,)
martinfleis commented 1 year ago

Well, this is probably not just a missing coverage but a bug, no?

jGaboardi commented 1 year ago

Well, this is probably not just a missing coverage but a bug, no?

Yes, maybe all of the above and more. That's the scary part...