Please add Z and M support to Shapefile read/write API.

[GoogleCodeExporter]

What steps will reproduce the problem?

1. Try to use the shapefile API to either read or write files with Z or M 
values. It will ignore them at best.

What is the expected output? What do you see instead?

The Z and M values should be read from the file during read and written during 
write.

What version of the product are you using? On what operating system?

1.12 on Windows 7

Please provide any additional information below.

I have implemented Z and M support and created a patch file with the fix 
(attached). I have tested the fix with unit test cases (in the patch) and also 
as part of our product where I now use the library to import/export shapefiles.

Original issue reported on code.google.com by tbe...@gmail.com on 9 Oct 2012 at 9:32

Attachments:

• ShapeFilePatch1_TrondBenum.patch

[GoogleCodeExporter]

Thank you Trond!
Did you add 
public double M; 
to GeoAPI.Geometries.Coordinate to make your patch work?

Original comment by felix.ob...@netcologne.de on 9 Oct 2012 at 10:01

• Added labels: Type-Enhancement
• Removed labels: Type-Defect

[GoogleCodeExporter]

Yes I did. Is that a potential problem with respect to that it is a
different open source project or something like that?

Trond

Original comment by tbe...@gmail.com on 9 Oct 2012 at 10:21

[GoogleCodeExporter]

It is an issue because there have been voices asking to remove even Z since it 
is not handled properly (except for triangulation) and tends to eat up memory 
space.

There is an open issue regarding the coordinate model 
(http://code.google.com/p/nettopologysuite/issues/detail?id=106)

Original comment by felix.ob...@netcologne.de on 9 Oct 2012 at 10:34

[GoogleCodeExporter]

Original comment by felix.ob...@netcologne.de on 9 Oct 2012 at 10:34

[GoogleCodeExporter]

@Trond: Do you use coord.M throughout your application?
If not it should be fairly easy to achive the same using some coordinate 
sequence implementation (e.g. DotSpatialAffineCoordinateSequence):

var m = seq.GetOrdinate(index, Ordinate.M);
seq.SetOrdinate(index, Ordinate.M, mvalue);

Original comment by felix.ob...@netcologne.de on 9 Oct 2012 at 10:46

[GoogleCodeExporter]

It was a while since i discussed the issue with M-values now.
But, I'm also one of those who need both M-values and Z-values in my 
applications.

Looking what ESRI does, one idea could be to implement the 
IPoint, IPointZ : IPoint ,IPointZM : IPointZ

One other this is to make the LinearReferencing-parts M-aware, so you can do 
ExtractPointAtMeasure(m), GetSubLineBetweenMeasures(m1,m2)

Original comment by peter.lo...@gmail.com on 9 Oct 2012 at 11:11

[GoogleCodeExporter]

At this point, in our application I have isolated the use of the
NetTopologySuite Coordinate class so we only set it up when exporting to
shapefile and then read it again when importing.

So for that purpose it is the same whether I access it via Coordinate.M or
via some ordinate on the Geometry class.

However, for the sake of having as little code as possible, and more
important less code paths to test, I think I would prefer to have
CoordinateZ, CoordinateM and CoordinateZM classes instead of the ordinate
approach though.

That would mean we have to make sure we don't have any places in the code
where we create a new Coordinate from another - we would have to add a Copy
member on the Coordinate class instead - so that we ensure that we carry the
Z/M values along. If we do that, I think the code should be pretty
manageable. I am a little skeptic to having to enhance all the different
geometry manipulation code out there to carry Z/M along - I prefer that the
Z/M values flows along with the coordinates (from a code maintenance
perspective) as that is where they logically belong (in my head at least
:)).

Trond

Original comment by tbe...@gmail.com on 9 Oct 2012 at 12:37

[GoogleCodeExporter]

FYI, If you guys that have been on this project for a while make a call and 
provide some guideance/decision as to what strategy we should take when adding 
the M support, then I can probably spend some time and adjust the 
implementation.

Original comment by tbe...@gmail.com on 10 Oct 2012 at 1:17

[GoogleCodeExporter]

I'd prefer a solution based on ICoordinateSequences

Original comment by felix.ob...@netcologne.de on 19 Oct 2012 at 7:31

[GoogleCodeExporter]

I finally have some time to look at this again.

I can modify it to use ICoordinateSequence but that probably means I need to
expose a ICoordinateSequence property off  IGeometry (so that given a
geometry you can get to the M value). Is that ok?

Or should I only expose it for each implementation?

Original comment by tbe...@gmail.com on 27 Nov 2012 at 12:27

[GoogleCodeExporter]

The specific geometry classes expose ICoordinateSequence as a property.
I don't known if M value makes sense on geometry base class?

Original comment by felix.ob...@netcologne.de on 27 Nov 2012 at 2:23

• Changed state: Started

[GoogleCodeExporter]

I have now removed the M value property from Coordinate and implemented this 
using ICoordinateSequence instead. The attached patch replaces the original 
patch (use only the attached one). 

I left the Z handling as is - since Z was already in place on the Coordinate 
class. However - if one decides to remove Z in the future, it will now be easy 
(for the shapefile implementation at least) to switch to Ordinate.Z on the 
Geometry instead as it will use the exact same pattern as for M.

This ICoordinateSequence approach is more memory efficient, my only concern is 
that it requires more careful thinking when modifying the Coordinates in a 
Geometry - since the M values will not follow the Coordinate automatically. We 
can probably live with that though - most people will not use M values anyway 
so I can understand the memory issue.

I solved the access issue for M values by adding the following method on 
IGeometry:

double[] GetOrdinates(Ordinate ordinate);

That way we only expose what we need to access additional ordinates in the api 
- and not the entire ICoordinateSequence (which would make this complex for 
multi parts geometries) and it follows the same pattern as the Coordinates get 
property on IGeometry.

Note that I have only implemented the M ordinate inside the 
CoordinateArraySequence as I did not think it makes sense to implement support 
for ordinates which we will not use. It should be straight forward to fix this 
in the future without having to change the api though.

When creating a shape with M values you have to use the existing 
ICoordinateSequence Create call on the geometry factory.

Please let me know if this patch gets included in the trunk, as in that case I 
want to also update our internal usage to use this new version. Thanks.

Original comment by tbe...@gmail.com on 28 Nov 2012 at 8:54

Attachments:

• ShapeFilePatch-UsingCoordinateSequenceForM.patch

[GoogleCodeExporter]

> I solved the access issue for M values by adding the following method on 
IGeometry:
> 
> double[] GetOrdinates(Ordinate ordinate);

I can life with that, any other comments?

Original comment by felix.ob...@netcologne.de on 28 Nov 2012 at 9:33

[GoogleCodeExporter]

>I can life with that
A agree

Original comment by diegogu...@gmail.com on 28 Nov 2012 at 9:40

[GoogleCodeExporter]

@Trond: Could you provide "with_M" shapefile? Thanks

Original comment by felix.ob...@netcologne.de on 28 Nov 2012 at 4:31

[GoogleCodeExporter]

See attachment. I also included the ZMtest file.

Original comment by tbe...@gmail.com on 29 Nov 2012 at 9:11

Attachments:

• NetTopologySuite.Samples.Shapefiles.zip

[GoogleCodeExporter]

I've applied a modified version of your patches. Could you please evaluate if 
it works for you as expected (r949)

Note: I have not changed the ICoordinateSequenceFactory interface, as I'm 
hesistant to do that. As a utility you can use CoordinateBuffer class to create 
your sequences. Be sure to either create your own coordinate sequence 
implementation or use the DotSpatialAffineCoordinateSequence and its factory 
class. That can handle Z- and M-values and should be memory efficient, too.

Original comment by felix.ob...@netcologne.de on 29 Nov 2012 at 2:06

• Changed state: Fixed