Merge pull request #520 from peterstace/remove_omit_invalid

Remove `OmitInvalid` constructor option

CHANGELOG.md

@@ -1,5 +1,10 @@
 # Changelog
 
+## Unreleased
+
+- **Breaking change**: Removes the `OmitInvalid` constructor option. Users of
+  `OmitInvalid` should manage this behaviour manually.
+
 ## v0.44.0
 
 2023-06-23

geom/ctor_options.go

@@ -20,36 +20,8 @@ func DisableAllValidations(o *ctorOptionSet) {
 	o.skipValidations = true
 }
 
-// OmitInvalid causes geometry constructors to omit any geometries or
-// sub-geometries that are invalid.
-//
-// The behaviour for each geometry type is:
-//
-// * Point: if the Point is invalid, then it is replaced with an empty Point.
-//
-// * MultiPoint: if a child Point is invalid, then it is replace with an empty
-// Point within the MultiPoint.
-//
-// * LineString: if the LineString is invalid (e.g. doesn't contain at least 2
-// distinct points), then it is replaced with an empty LineString.
-//
-// * MultiLineString: if a child LineString is invalid, then it is replaced
-// with an empty LineString within the MultiLineString.
-//
-// * Polygon: if the Polygon is invalid (e.g. self intersecting rings or rings
-// that intersect in an invalid way), then it is replaced with an empty
-// Polygon.
-//
-// * MultiPolygon: if a child Polygon is invalid, then it is replaced with an
-// empty Polygon within the MultiPolygon. If two child Polygons  interact in an
-// invalid way, then the MultiPolygon is replaced with an empty MultiPolygon.
-func OmitInvalid(o *ctorOptionSet) {
-	o.omitInvalid = true
-}
-
 type ctorOptionSet struct {
 	skipValidations bool
-	omitInvalid     bool
 }
 
 func newOptionSet(opts []ConstructorOption) ctorOptionSet {

geom/ctor_options_test.go

@@ -1,7 +1,6 @@
 package geom_test
 
 import (
-	"math"
 	"strconv"
 	"testing"
 
@@ -44,91 +43,3 @@ func TestDisableValidation(t *testing.T) {
 		})
 	}
 }
-
-func TestOmitInvalid(t *testing.T) {
-	for i, tt := range []struct {
-		input  string
-		output string
-	}{
-		{
-			"LINESTRING(1 1)",
-			"LINESTRING EMPTY",
-		},
-		{"LINESTRING Z (1 1 1)", "LINESTRING Z EMPTY"},
-		{"LINESTRING M (1 1 1)", "LINESTRING M EMPTY"},
-		{"LINESTRING ZM (1 1 1 1)", "LINESTRING ZM EMPTY"},
-		{
-			"LINESTRING(2 2,2 2)",
-			"LINESTRING EMPTY",
-		},
-		{
-			"MULTILINESTRING((3 3))",
-			"MULTILINESTRING(EMPTY)",
-		},
-		{"MULTILINESTRING Z ((3 3 0))", "MULTILINESTRING Z (EMPTY)"},
-		{"MULTILINESTRING M ((3 3 0))", "MULTILINESTRING M (EMPTY)"},
-		{"MULTILINESTRING ZM ((3 3 0 0))", "MULTILINESTRING ZM (EMPTY)"},
-		{
-			"MULTILINESTRING((4 4,5 5),(6 6,6 6))",
-			"MULTILINESTRING((4 4,5 5),EMPTY)",
-		},
-		{
-			"MULTILINESTRING((7 7,7 7),(8 8,9 9))",
-			"MULTILINESTRING(EMPTY,(8 8,9 9))",
-		},
-		{
-			"POLYGON((0 0,1 1,0 1,1 0,0 0))",
-			"POLYGON EMPTY",
-		},
-		{"POLYGON Z ((0 0 0,1 1 0,0 1 0,1 0 0,0 0 0))", "POLYGON Z EMPTY"},
-		{"POLYGON M ((0 0 0,1 1 0,0 1 0,1 0 0,0 0 0))", "POLYGON M EMPTY"},
-		{"POLYGON ZM ((0 0 0 0,1 1 0 0,0 1 0 0,1 0 0 0,0 0 0 0))", "POLYGON ZM EMPTY"},
-		{
-			"MULTIPOLYGON(((0 0,1 1,0 1,1 0,0 0)))",
-			"MULTIPOLYGON(EMPTY)",
-		},
-		{
-			"MULTIPOLYGON(((0 0,1 1,0 1,1 0,0 0)),((0 0,0 1,1 0,0 0)))",
-			"MULTIPOLYGON(EMPTY,((0 0,0 1,1 0,0 0)))",
-		},
-		{
-			"MULTIPOLYGON(((0 0,0 1,1 0,0 0)),((0 0,1 1,0 1,1 0,0 0)))",
-			"MULTIPOLYGON(((0 0,0 1,1 0,0 0)),EMPTY)",
-		},
-		{
-			"MULTIPOLYGON(((0 0,0 2,2 2,2 0,0 0)),((1 1,1 3,3 3,3 1,1 1)))",
-			"MULTIPOLYGON EMPTY",
-		},
-		{
-			"GEOMETRYCOLLECTION(LINESTRING(2 2,2 2))",
-			"GEOMETRYCOLLECTION(LINESTRING EMPTY)",
-		},
-		{
-			"GEOMETRYCOLLECTION(GEOMETRYCOLLECTION(LINESTRING(2 2,2 2)))",
-			"GEOMETRYCOLLECTION(GEOMETRYCOLLECTION(LINESTRING EMPTY))",
-		},
-	} {
-		t.Run(strconv.Itoa(i), func(t *testing.T) {
-			g := geomFromWKT(t, tt.input, geom.OmitInvalid)
-			expectGeomEq(t, g, geomFromWKT(t, tt.output))
-		})
-	}
-}
-
-func TestOmitInvalidDirectPointConstruction(t *testing.T) {
-	// This test is separate from the cases specified using WKTs because it's
-	// not possible to unmarshal an invalid Point WKT due to it not matching
-	// the WKT grammar.
-	for _, ct := range []geom.CoordinatesType{geom.DimXYZ, geom.DimXYM, geom.DimXYZM} {
-		t.Run(ct.String(), func(t *testing.T) {
-			coords := geom.Coordinates{
-				XY:   geom.XY{X: math.NaN(), Y: math.NaN()},
-				Type: ct,
-			}
-			pt, err := geom.NewPoint(coords, geom.OmitInvalid)
-			expectNoErr(t, err)
-			expectTrue(t, pt.IsEmpty())
-			expectCoordinatesTypeEq(t, pt.CoordinatesType(), ct)
-		})
-	}
-}

geom/type_line_string.go

@@ -26,23 +26,12 @@ func NewLineString(seq Sequence, opts ...ConstructorOption) (LineString, error)
 	if ctorOpts.skipValidations {
 		return LineString{seq}, nil
 	}
-	if ctorOpts.omitInvalid {
-		return newLineStringWithOmitInvalid(seq), nil
-	}
-
 	if err := validateLineStringSeq(seq); err != nil {
 		return LineString{}, err
 	}
 	return LineString{seq}, nil
 }
 
-func newLineStringWithOmitInvalid(seq Sequence) LineString {
-	if err := validateLineStringSeq(seq); err != nil {
-		return LineString{}.ForceCoordinatesType(seq.CoordinatesType())
-	}
-	return LineString{seq}
-}
-
 func validateLineStringSeq(seq Sequence) error {
 	if seq.Length() == 0 {
 		return nil
@@ -440,7 +429,11 @@ func (s LineString) Simplify(threshold float64) LineString {
 	seq := s.Coordinates()
 	floats := ramerDouglasPeucker(nil, seq, threshold)
 	seq = NewSequence(floats, seq.CoordinatesType())
-	return newLineStringWithOmitInvalid(seq)
+	ls, err := NewLineString(seq)
+	if err != nil {
+		return LineString{}.ForceCoordinatesType(s.CoordinatesType())
+	}
+	return ls
 }
 
 // InterpolatePoint returns a Point interpolated along the LineString at the

geom/type_multi_polygon.go

@@ -45,9 +45,6 @@ func NewMultiPolygon(polys []Polygon, opts ...ConstructorOption) (MultiPolygon,
 
 	ctorOpts := newOptionSet(opts)
 	if err := validateMultiPolygon(polys, ctorOpts); err != nil {
-		if ctorOpts.omitInvalid {
-			return MultiPolygon{}, nil
-		}
 		return MultiPolygon{}, err
 	}
 	return MultiPolygon{polys, ctype}, nil

geom/type_point.go

@@ -25,9 +25,6 @@ func NewPoint(c Coordinates, opts ...ConstructorOption) (Point, error) {
 		return newUncheckedPoint(c), nil
 	}
 	if err := c.XY.validate(); err != nil {
-		if os.omitInvalid {
-			return NewEmptyPoint(c.Type), nil
-		}
 		return Point{}, validationError{err.Error()}
 	}
 	return newUncheckedPoint(c), nil

geom/type_polygon.go

@@ -49,9 +49,6 @@ func NewPolygon(rings []LineString, opts ...ConstructorOption) (Polygon, error)
 
 	ctorOpts := newOptionSet(opts)
 	if err := validatePolygon(rings, ctorOpts); err != nil {
-		if ctorOpts.omitInvalid {
-			return Polygon{}.ForceCoordinatesType(ctype), nil
-		}
 		return Polygon{}, err
 	}
 	return Polygon{rings, ctype}, nil

geom/validation_test.go

@@ -52,30 +52,6 @@ func TestDisableAllPointValidations(t *testing.T) {
 	expectNoErr(t, err)
 }
 
-func TestOmitInvalidPoint(t *testing.T) {
-	t.Run("DimXY", func(t *testing.T) {
-		c := xy(2, math.NaN())
-
-		_, err := NewPoint(c)
-		expectErr(t, err)
-
-		pt, err := NewPoint(c, OmitInvalid)
-		expectNoErr(t, err)
-		expectTrue(t, pt.IsEmpty())
-	})
-	t.Run("DimXYZ", func(t *testing.T) {
-		c := Coordinates{Type: DimXYZ, XY: XY{2, math.NaN()}}
-
-		_, err := NewPoint(c)
-		expectErr(t, err)
-
-		pt, err := NewPoint(c, OmitInvalid)
-		expectNoErr(t, err)
-		expectTrue(t, pt.IsEmpty())
-		expectCoordinatesTypeEq(t, pt.CoordinatesType(), DimXYZ)
-	})
-}
-
 func TestLineStringValidationInvalidFromRawCoords(t *testing.T) {
 	nan := math.NaN()
 	inf := math.Inf(+1)
@@ -97,9 +73,6 @@ func TestLineStringValidationInvalidFromRawCoords(t *testing.T) {
 			expectErr(t, err)
 			_, err = NewLineString(seq, DisableAllValidations)
 			expectNoErr(t, err)
-			ls, err := NewLineString(seq, OmitInvalid)
-			expectNoErr(t, err)
-			expectTrue(t, ls.IsEmpty())
 		})
 	}
 }