Repository: opendatacube/odc-geo (python)

Issue:
negative size shape is allowed
I noticed that values of `shape` can be negative, this doesn't seem desirable, it has the effect of flipping the transform as expected. 

```python
from odc.geo.geobox import GeoBox
ex = [-120, -100, 35, 55]

g = GeoBox.from_bbox([140, -43, 160, -23],
   "EPSG:4326", 
   shape = [-1, 1])
g
#GeoBox((-1, 1), Affine(20.0, 0.0, 140.0,
#       0.0, 20.0, -60.0), CRS('EPSG:4326'))
```

I only noticed because I wanted to do the GDAL thing of setting one of the dimensions to zero, whereby you only need to use the shape of one side and it helpfully calculates the other from the aspect ratio of the extent. This is nice because it gives a very short heuristic for "about this detailed" that matches how images work, rather than having  to specify a specific resolution or do gymnastics to work out the ratio.   

So, maybe negative shape should be disallowed, and if it's of interest I could PR the `shape = [0, n]` or `shape = [n, 0]`. 

Thanks!  

Pull request:
Disallow zero and negative size shapes
Attempting to create a GeoBox with an x or y dimension of zero results in a zero-division error because it's not possible to calculate the resolution of the box.  Attempting to create a GeoBox with a negative dimension works but flips the transform.  That's confusing so we'd like to disallow it.

This PR raises exceptions when these situations occur; I've also added tests to confirm functionality.

Resolves #136.

Patch:
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index 79eecc2..466afa0 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -1,6 +1,6 @@
 repos:
     - repo: https://github.com/pre-commit/pre-commit-hooks
-      rev: v4.5.0
+      rev: v4.6.0
       hooks:
         - id: end-of-file-fixer
         - id: trailing-whitespace
diff --git a/odc/geo/geobox.py b/odc/geo/geobox.py
index a006cd3..5e9433b 100644
--- a/odc/geo/geobox.py
+++ b/odc/geo/geobox.py
@@ -122,6 +122,11 @@ class GeoBoxBase:
         self._extent: Optional[Geometry] = None
         self._lazy_ui = None
 
+        if self.shape.x < 0 or self.shape.y < 0:
+            raise ValueError(
+                f"Got shape {self._shape.yx!r}: negative sizes are not allowed."
+            )
+
     @property
     def width(self) -> int:
         """Width in pixels (nx)."""
@@ -576,6 +581,17 @@ class GeoBox(GeoBoxBase):
 
         shape = shape_(shape)
         nx, ny = shape.wh
+
+        if nx == 0 or ny == 0:
+            raise ValueError(
+                f"Got zero size in ({nx}, {ny}): cannot calculate resolution."
+            )
+
+        if nx < 0 or ny < 0:
+            raise ValueError(
+                f"Got negative size in ({nx}, {ny}): negative dimensions are not allowed."
+            )
+
         rx = bbox.span_x / nx
         ry = -bbox.span_y / ny
 


Test patch:
diff --git a/tests/test_geobox.py b/tests/test_geobox.py
index 36a3ac7..2a6b212 100644
--- a/tests/test_geobox.py
+++ b/tests/test_geobox.py
@@ -685,3 +685,22 @@ def test_explore_geobox(geobox):
     assert (
         sum(isinstance(child, GeoJson) for child in m_external._children.values()) == 2
     )
+
+
+@pytest.mark.parametrize(
+    ("shape", "allowed"),
+    [
+        pytest.param((100, 100), True, id="positive xy: normal"),
+        pytest.param((0, 100), False, id="zero x: can't calculate resolution"),
+        pytest.param((100, 0), False, id="zero y: can't calculate resolution"),
+        pytest.param((-100, 100), False, id="negative x: not allowed"),
+        pytest.param((100, -100), False, id="negative y: not allowed"),
+        pytest.param((-100, -100), False, id="both negative: not allowed"),
+    ],
+)
+def test_shape(shape, allowed):
+    if allowed:
+        GeoBox.from_bbox((-3, -4, 3, 4), epsg4326, shape=shape)
+    else:
+        with pytest.raises(ValueError):
+            GeoBox.from_bbox((-3, -4, 3, 4), epsg4326, shape=shape)