Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace deprecated To/From WKT/WKB functions with Reader/Writers #63

Closed
mwtoews opened this issue Jul 9, 2013 · 11 comments
Closed

Replace deprecated To/From WKT/WKB functions with Reader/Writers #63

mwtoews opened this issue Jul 9, 2013 · 11 comments
Assignees
Milestone

Comments

@mwtoews
Copy link
Member

mwtoews commented Jul 9, 2013

According to the GEOS C API, GEOSGeomToWKT and GEOSGeomFromWKT (used in ctypes_declarations.py) are deprecated (since 3.0.0-CAPI-1.4.1), and should use the new WKT Reader and Writer functions. The same applies to WKB.

This idea was sparked in #32

@mwtoews
Copy link
Member Author

mwtoews commented Jul 10, 2013

I've been poking around the reader and writer APIs, and they are actually straight-forward, and offers nice flexibility. See the API docs and WKTWriterTest.cpp to get an idea of the writer's for usage and examples of output. I think it would be a good idea to make a WKT writer a persistent object, with properties exposed in, say, shapely.wkt.writer ( bold as default):

  • trim : { True, False}, e.g. POINT (1 2) vs POINT (1.0000000000000000 2.0000000000000000)
  • output_dim : {2, 3}, e.g. a 3D point: POINT (1 2) vs POINT Z (1 2 3) (note: 2D shapes are always represented as 2D, regardless of this setting)
  • rounding_precision : which is -1 for unset (default), otherwise any positive integer, e.g., with 3 will turn POINT(-117.1234567 33.1234567) into POINT (-117.123 33.123)

I don't know if Old3D would be useful, e.g., compare POINT Z (1 2 3) with POINT (1 2 3) (old format).

@sgillies
Copy link
Contributor

Thanks for digging in to this @mwtoews. I'm +1 on adding a WKT writer class, but -1 on a module level instance of it. I regret the module state that I've already saddled on Shapely, and don't want to add more. A new class actually gives us a way out: wkt.dump (and dumps) could create their own writer instance as needed or take one from the caller (as with json.dump) or programs could create and reuse their own writers.

@mwtoews
Copy link
Member Author

mwtoews commented Jul 15, 2013

Moving the classes/objects off shapely.wkt is no big deal. I think there should be four classes in shapely.geos: WKTReader, WKTWriter, WKBReader, and WKBWriter. The readers don't have any properties.

WKBWriter has a few get/set properties: output_dimension, byte_order, and include_srid. Also, the WKTWriter get/set properties mentioned in my comment above are only available with GEOS >= 3.3.0.

In shapely.geos, the reader/writer objects are created, and are persistent until cleanup().

BTW, I'm half done implementing this, but I haven't pushed any code out yet. I'm mostly fumbling around with ctypes and looking at the GEOS C API versions.

@larsbutler
Copy link

@mwtoews For what it's worth, I'm working on a lib which provides GeoJSON <--> WKB/WKT, including support for 2d, 3d, and 4d geometry. It's pure Python, so no mucking around with the GEOS API. https://github.com/larsbutler/geomet

It's still a work in progress, though--I don't have all of the geometry types implemented.

@mwtoews
Copy link
Member Author

mwtoews commented Jul 15, 2013

Thanks @larsbutler I'll have a look. @sgillies suggested using a Python WKT/B implementation in #32

I'm not sure if there are performance benefits with the GEOS C API reader/writer, but there is certainly more flexibility and simplicity with implementing a pure Python solution.

@ghost ghost assigned sgillies Jul 23, 2013
@mwtoews
Copy link
Member Author

mwtoews commented Jul 24, 2013

Mostly implemented here: mwtoews@7bd61da

@mwtoews
Copy link
Member Author

mwtoews commented Jul 26, 2013

I've been thinking about doing a few things. First, the default reader/writers are created to:

  • shapely.geos.lgeos.wkt_reader
  • shapely.geos.lgeos.wkt_writer
  • shapely.geos.lgeos.wkb_reader
  • shapely.geos.lgeos.wkb_writer

To change the properties for the default writers, one would need to:

from shapely.geos import lgeos

lgeos.wkt_writer.trim = False
lgeos.wkb_writer.big_endian = False

Should a copy of the default writer objects be put somewhere else? What about the default writers' default output settings? Currently, I have it to set a different WKT output for GEOS >=3.3.0, using trim=True and output_dimension=3. Older versions of GEOS can't set the WKT writer options. WKB output is the same for all versions of GEOS (now using output_dimension=3).


Second, to allow keywords to WK[TB]Writer's __init__ method (via **kwargs), to establish defaults (where supported by the GEOS version and writer). So, it would be used like ..

wkt_writer = WKTWriter(lgeos, trim=True, output_dimension=3)
wkb_writer = WKBWriter(lgeos, output_dimension=3, big_endian=False)

Furthermore, allow these keywords to be used by shapely.wk[tb].dumps:

from shapely.geometry import Point
from shapely.wkt import dumps
a = Point(1,2,3)
dumps(a)  # POINT Z (1 2 3)
dumps(a, trim=True, output_dimension=3)  # POINT Z (1 2 3)
dumps(a, trim=True, output_dimension=2)  # POINT (1 2)
dumps(a, trim=False, output_dimension=2)  # POINT (1.0000000000000000 2.0000000000000000)
a.wkt  # POINT Z (1 2 3)  # using default writer

Here, dumps would create a new writer, set output options, write data, then it would destroy the writer. The default writer is an independent object, with different default settings (discussed above). Looking at json.dumps, it looks normal to have several keyword parameters for the writer implementation.

@georgthegreat
Copy link

It would be nice to have an ability to limit number of decimal digits in wkt.
As far, as I understood, trim parameters actually trims only trailing zeroes, while optional conversion from 1.124235345435345345354353 to 1.1242 also seems to be useful for me.

Unfortunately, I don't know if geos is capable if such trim.

@mwtoews
Copy link
Member Author

mwtoews commented Nov 15, 2013

@georgthegreat under the working code in my https://github.com/mwtoews/shapely/tree/v13 branch this could be done by setting rounding_precision for the WKTWriter to 4. E.g., modify the default writer:

from shapely.geos import lgeos
lgeos.wkt_writer.rounding_precision = 4

@georgthegreat
Copy link

Thanks!

This was referenced Nov 17, 2013
Merged
@olt
Copy link
Contributor

olt commented Nov 18, 2013

Some random remarks:

  • please do not use __xxx__, this is reserved for special methods in Python. One underline should be enough to mark it as a private variable.
  • should WKT/BReader/Writer become an official API? I would make the first argument default to geos.lgeos, since all other functions and constructors do not require a geos handle.
  • right now setting writer options with 3.2 will raise AttributeError, don't know the user should get an exception with a description (GEOS >=3.3 required)
  • I don't like having global state (i.e. the wkt_writer). I would make the default reader/writer private to not encourage users to modify it. If they need to change the format, then they should use dumps or a custom writer.

sgillies pushed a commit that referenced this issue Nov 19, 2013
- New classes in geos.py, with persistent objects in geos.lgeos
- Use __del__() methods for classes that need to be cleaned up
- Move atexit register to a decorator at the bottom
- Writers implement get/set properties, where appropriate
- Use a different defaults for WKT Writer for GEOS 3.3.0 and up, to
  output 3 dimensions and trim extra decimals
- Set WKB Writer to have default output dimensions of 3 (#64)
- Deprecate older To/From WKT/WKB functions in geometry.base, particularly:
  - geom_from_wkt(data)
  - geom_to_wkt(ob)
  - geom_from_wkb(data)
  - geom_to_wkb(ob)
  - to_wkb(self)
  - to_wkt(self)
 - Keeping deserialize_wkb(data) for now, since I can't figure out how
   to avoid circular imports using the reader; only used for 'EMPTY'
 - Geometry objects now have a 'wkb_hex' property, for convenience
 - Test suites run using older WKT formatting, so these don't need to
   be modified; all tests appear to pass
jorisvandenbossche added a commit to jorisvandenbossche/shapely that referenced this issue Nov 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants