opengeospatial / geopackage

An asciidoc version of the GeoPackage specification for easier collaboration
https://www.geopackage.org
Other
264 stars 71 forks source link

CurvePolygon should be abstract #178

Closed jyutzler closed 8 years ago

jyutzler commented 8 years ago

From @kryden

Figure 2 – core geometry model diagram – I believe that the class “CurvePolygon” should be <<abstract>>.

Fixing this would also give us a chance to put the source files for the figures under version control where they belong. Only 3 of the 6 figures are currently in there.

pepijnve commented 8 years ago

I don't think that's correct. That diagram is taken from the SQL/MM spec where, iirc, CurvePolygon is an instantiable type. I don't have access to the spec anymore so can't double check.

jyutzler commented 8 years ago

I've asked @pdaisey to double-check.

pdaisey commented 8 years ago

As Pepijn said, the diagrams and definitions were taken from ISO/IEC 13249-3.

In GeoPackage Clause 2.1.1:

A brief description of each geometry type is provided below. A more detailed description can be found in ISO 13249-3[12].

I just have the FDIS version of 13249-3.

In Clause 4.1 Geometry Types:

ST_Geometry, ST_Curve, and ST_Surface are not instantiable types. No constructor functions are defined for these types.

ST_Point, ST_LineString, ST_CircularString, ST_CompoundCurve, ST_CurvePolygon, ST_Polygon, ST_Triangle, ST_PolyhdrlSurface, ST_TIN, ST_GeomCollection, ST_MultiPoint, ST_MultiCurve, ST_MultiLineString, ST_MultiSurface, and ST_MultiPolygon are instantiable and have constructor functions.

In Clause 4.9 4.2.9 ST_CurvePolygon:

The ST_CurvePolygon type is a subtype of ST_Surface. The ST_CurvePolygon type is instantiable. An ST_CurvePolygon value is a planar surface consisting of a single patch, defined by one exterior boundary and zero or more interior boundaries. Each interior boundary defines a hole in the ST_CurvePolygon value.

Per those definitions the diagram is correct to show ST_CurvePolygon as instantiable.

On 12/9/2015 9:44 AM, Jeff Yutzler wrote:

I've asked @pdaisey https://github.com/pdaisey to double-check.

— Reply to this email directly or view it on GitHub https://github.com/opengeospatial/geopackage/issues/178#issuecomment-163267907.

KRyden commented 8 years ago

Let’s leave it alone – references below indicate no issue with the existing diagram.

Thanks Keith

From: pdaisey [mailto:notifications@github.com] Sent: Wednesday, December 9, 2015 7:57 AM To: opengeospatial/geopackage Cc: Keith Ryden Subject: Re: [geopackage] CurvePolygon should be abstract (#178)

As Pepijn said, the diagrams and definitions were taken from ISO/IEC 13249-3.

In GeoPackage Clause 2.1.1:

A brief description of each geometry type is provided below. A more detailed description can be found in ISO 13249-3[12].

I just have the FDIS version of 13249-3.

In Clause 4.1 Geometry Types:

ST_Geometry, ST_Curve, and ST_Surface are not instantiable types. No constructor functions are defined for these types.

ST_Point, ST_LineString, ST_CircularString, ST_CompoundCurve, ST_CurvePolygon, ST_Polygon, ST_Triangle, ST_PolyhdrlSurface, ST_TIN, ST_GeomCollection, ST_MultiPoint, ST_MultiCurve, ST_MultiLineString, ST_MultiSurface, and ST_MultiPolygon are instantiable and have constructor functions.

In Clause 4.9 4.2.9 ST_CurvePolygon:

The ST_CurvePolygon type is a subtype of ST_Surface. The ST_CurvePolygon type is instantiable. An ST_CurvePolygon value is a planar surface consisting of a single patch, defined by one exterior boundary and zero or more interior boundaries. Each interior boundary defines a hole in the ST_CurvePolygon value.

Per those definitions the diagram is correct to show ST_CurvePolygon as instantiable.

On 12/9/2015 9:44 AM, Jeff Yutzler wrote:

I've asked @pdaisey https://github.com/pdaisey to double-check.

— Reply to this email directly or view it on GitHub https://github.com/opengeospatial/geopackage/issues/178#issuecomment-163267907.

— Reply to this email directly or view it on GitHubhttps://github.com/opengeospatial/geopackage/issues/178#issuecomment-163301393.