Cannot use GeometryCollection when shapely is installed

[liskin]

Similar to #68.

When shapely is installed, GeometryCollection can't be used:

from fastkml import kml
p = kml.Placemark(styleUrl="#normal")
p.geometry = {'type': "GeometryCollection", 'geometries': []}
p.to_string()

This fails with ValueError("Illegal geometry type."), but it works just fine when fastkml uses pygeoif.

I'm guessing it's because

fastkml/fastkml/geometry.py

Line 43 in 9f24722

from pygeoif.geometry import GeometryCollection

and then

fastkml/fastkml/geometry.py

Line 275 in 9f24722

elif isinstance(self.geometry, GeometryCollection):

fails because

fastkml/fastkml/geometry.py

Line 128 in 9f24722

self.geometry = asShape(geometry)

creates a shapely GeometryCollection, not pygeoif one.

[cleder]

#137 drops shapely support

[liskin]

Oh, but why? It was really practical being able to use shapely objects directly. 😿

[cleder]

Mainly to support pygeoif 1.0, type annotations, and to get rid of some edge case bugs.
You can use shapely.geometry.shape to convert between pygeoif and shapely geometries.

[liskin]

Okay, fair enough, I'll give shapely.geometry.shape (or in my case the reverse, shapely.geometry.mapping) a try. Thanks for the answer.

[cleder]

Maybe we could inject a geometry transformer function when constructing the KML, (e.g shapely.geometry.shape) so the geometry can be returned in the preferred format.

[liskin]

I finally got around to testing my code with fastkml 1.0a1 and it turns out it just works. shapely geometry is automatically converted to pygeoif geometry via __geo_interface__ and everything just works. So… nevermind and sorry for the noise. :-)

[cleder]

Thanks for testing and letting me know, :-) Much appreciated!

[liskin]

Seems like __geo_interface__ stuff doesn't work any more in 1.0.alpha.6. Looks like it's now necessary to manually convert even pygeoif objects to fastkml.geometry objects otherwise one gets errors like these:
AttributeError: 'GeometryCollection' object has no attribute 'etree_element'

I wonder if that's intentional?

[cleder]

No it is not intentional.
Can you supply an example to reproduce this?

[liskin]

Sure:

#!/usr/bin/env python3

import fastkml
import pygeoif.geometry

g = pygeoif.geometry.Point(1, 1)
if fastkml.__version__ == "1.0a4":
    p = fastkml.kml.Placemark()
    p.geometry = g
else:
    p = fastkml.kml.Placemark(geometry=g)
print(p.to_string())

$ python3 -m venv venv-fastkml-a4
$ python3 -m venv venv-fastkml-a6
$ ./venv-fastkml-a4/bin/pip install 'fastkml == 1.0a4'
$ ./venv-fastkml-a6/bin/pip install 'fastkml == 1.0a6'
$ ./venv-fastkml-a4/bin/python fastkml-interface.py
/tmp/venv-fastkml-a4/lib/python3.11/site-packages/fastkml/config.py:37: UserWarning: Package `lxml` missing. Pretty print will be disabled
  warnings.warn("Package `lxml` missing. Pretty print will be disabled")
<kml:Placemark xmlns:kml="http://www.opengis.net/kml/2.2"><kml:visibility>1</kml:visibility><kml:Point><kml:coordinates>1.000000,1.000000</kml:coordinates></kml:Point></kml:Placemark>
$ ./venv-fastkml-a6/bin/python fastkml-interface.py 
/tmp/venv-fastkml-a6/lib/python3.11/site-packages/fastkml/config.py:37: UserWarning: Package `lxml` missing. Pretty print will be disabled
  warnings.warn("Package `lxml` missing. Pretty print will be disabled")
Traceback (most recent call last):
  File "/tmp/fastkml-interface.py", line 13, in <module>
    print(p.to_string())
          ^^^^^^^^^^^^^
  File "/tmp/venv-fastkml-a6/lib/python3.11/site-packages/fastkml/base.py", line 116, in to_string
    self.etree_element(
  File "/tmp/venv-fastkml-a6/lib/python3.11/site-packages/fastkml/kml.py", line 1757, in etree_element
    element.append(self._geometry.etree_element())
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'Point' object has no attribute 'etree_element'

So yeah I can pretty confidently say the examples from https://fastkml.readthedocs.io/en/latest/usage_guide.html no longer work :-)

[cleder]

There was a lot of refactoring in fastkml.geometry (it has developed into a mess, what was the developer thinking 🤯 ) Check what you pass against the type annotations, this probably changed during this refactor.
Thanks for your support, I will take a look, writing a guide migrating from 0 to 1 is on my radar, just not a priority right now

[cleder]

#239 is probably the place to look

[cleder]

closing this in favour of #264

[cleder]

@liskin the commit brings back the option to create a feature with just a geometry to support backwards compatibility

[liskin]

@liskin the commit brings back the option to create a feature with just a geometry to support backwards compatibility

Right, it does indeed. ♥
Unfortunately now one can't use fastkml.geometry objects for the Placemark geometry any more. Not sure if that's intended… 🙂
(I mean I don't want that, I want to be passing shapely/pygeoif data, but I think you perhaps wanted to support both?)

And then there are some vaguely related issues:

• geometry needs to be passed to the Placemark constructor because it's a read-only property without a setter
• style_url, on the other hand, can now only be set using the property setter, as the constructor argument assumes it's a StyleUrl class, whereas the property setter converts it from a string

And then I commented on 2 commits of yours pointing out other random breakages (XML namespaces, NetworkLink having no Link).

[liskin]

These are the changes I had to make to make my project's tests pass locally:

• 8ba227a
• 3222469
• liskin/liscopridge@5dbd6f3

Shall I open a PR from develop...liskin:fastkml:fixes?

[cleder]

PRs are always welcome ;-)

[cleder]

The current develop (I have not cut a release for this yet) reintroduces the ability to pass a shapely or pygeoif (or anything else that supports the __geo_interface__ protocol) as the geometry to placemark, or a fastkml.geometry kml object as kml_geometry.

I can reintroduce a geometry property getter and setter on feature if this is needed (although probably under another name to avoid confusion, like geo_geometry)

[liskin]

The current develop (I have not cut a release for this yet) reintroduces the ability to pass a shapely or pygeoif (or anything else that supports the __geo_interface__ protocol) as the geometry to placemark, or a fastkml.geometry kml object as kml_geometry.

Oh, I see, I somehow missed the kml_geometry bit. That's all right then.

I can reintroduce a geometry property getter and setter on feature if this is needed (although probably under another name to avoid confusion, like geo_geometry)

Not really needed, I just found it a bit confusing that old versions of fastkml accepted style_url (or even styleUrl back then I think) as constructor argument and geometry was passed via a property setter, and now in 1.0a9 it's the other way round. So… just make sure it's documented when you eventually make a release 🙂

[cleder]

I made some 'questionable' decisions for shortcuts 10 years ago, based on a use case I had back then. I never imagined it would have north of 100K downloads a month, in the first few years I was proud when it had 75 per month (IIRC) 😃

Version 1.0 should be more consistent, easier to work with and to contribute to.

I appreciate your continuing contributions, your comments are very valuable to give me insights what users want to get out of this package 👍