From 3c357d4bf92d499f52b9694eb77c5d4fbacb428d Mon Sep 17 00:00:00 2001 From: Evan Derickson <3587185+dericke@users.noreply.github.com> Date: Sun, 29 Nov 2020 13:00:18 -0800 Subject: [PATCH 1/5] Use list comprehensions to shorten code Remove unneeded conditionals before for loops --- fastkml/geometry.py | 71 ++++++++++++++++++++------------------------- 1 file changed, 32 insertions(+), 39 deletions(-) diff --git a/fastkml/geometry.py b/fastkml/geometry.py index 936e8f8d..e8d2d7bc 100644 --- a/fastkml/geometry.py +++ b/fastkml/geometry.py @@ -164,8 +164,8 @@ def _etree_coordinates(self, coordinates): tuples = ('%f,%f' % tuple(c) for c in coordinates) elif len(coordinates[0]) == 3: # if clampToGround: - # if the altitude is ignored anyway, we may as well - # ignore the z-value + # if the altitude is ignored anyway, we may as well + # ignore the z-value # tuples = ('%f,%f' % tuple(c[:2]) for c in coordinates) # else: tuples = ('%f,%f,%f' % tuple(c) for c in coordinates) @@ -322,9 +322,10 @@ def _get_coordinates(self, element): # spaces. Clean up badly formatted tuples by stripping # space following commas. latlons = re.sub(r', +', ',', coordinates.text.strip()).split() - coords = [] - for latlon in latlons: - coords.append([float(c) for c in latlon.split(',')]) + coords = [ + [float(c) for c in latlon.split(',')] + for latlon in latlons + ] return coords def _get_linear_ring(self, element): @@ -350,9 +351,8 @@ def _get_geometry(self, element): outer_boundary = element.find('%souterBoundaryIs' % self.ns) ob = self._get_linear_ring(outer_boundary) inner_boundaries = element.findall('%sinnerBoundaryIs' % self.ns) - ibs = [] - for inner_boundary in inner_boundaries: - ibs.append(self._get_linear_ring(inner_boundary)) + ibs = [self._get_linear_ring(inner_boundary) + for inner_boundary in inner_boundaries] return Polygon(ob, ibs) if element.tag == ('%sLinearRing' % self.ns): coords = self._get_coordinates(element) @@ -364,49 +364,42 @@ def _get_multigeometry(self, element): geoms = [] if element.tag == ('%sMultiGeometry' % self.ns): points = element.findall('%sPoint' % self.ns) - if points: - for point in points: - self._get_geometry_spec(point) - geoms.append(Point(self._get_coordinates(point)[0])) + for point in points: + self._get_geometry_spec(point) + geoms.append(Point(self._get_coordinates(point)[0])) linestrings = element.findall('%sLineString' % self.ns) - if linestrings: - for ls in linestrings: - self._get_geometry_spec(ls) - geoms.append(LineString(self._get_coordinates(ls))) + for ls in linestrings: + self._get_geometry_spec(ls) + geoms.append(LineString(self._get_coordinates(ls))) polygons = element.findall('%sPolygon' % self.ns) - if polygons: - for polygon in polygons: - self._get_geometry_spec(polygon) - outer_boundary = polygon.find( - '%souterBoundaryIs' % self.ns - ) - ob = self._get_linear_ring(outer_boundary) - inner_boundaries = polygon.findall( - '%sinnerBoundaryIs' % self.ns - ) - ibs = [] - for inner_boundary in inner_boundaries: - ibs.append(self._get_linear_ring(inner_boundary)) - geoms.append(Polygon(ob, ibs)) + for polygon in polygons: + self._get_geometry_spec(polygon) + outer_boundary = polygon.find( + '%souterBoundaryIs' % self.ns + ) + ob = self._get_linear_ring(outer_boundary) + inner_boundaries = polygon.findall( + '%sinnerBoundaryIs' % self.ns + ) + ibs = [self._get_linear_ring(inner_boundary) + for inner_boundary in inner_boundaries] + geoms.append(Polygon(ob, ibs)) linearings = element.findall('%sLinearRing' % self.ns) if linearings: for lr in linearings: self._get_geometry_spec(lr) geoms.append(LinearRing(self._get_coordinates(lr))) - if len(geoms) > 0: - geom_types = [] - for geom in geoms: - geom_types.append(geom.geom_type) - geom_types = list(set(geom_types)) + if geoms: + geom_types = {geom.geom_type for geom in geoms} if len(geom_types) > 1: return GeometryCollection(geoms) - if geom_types[0] == 'Point': + if 'Point' in geom_types: return MultiPoint(geoms) - elif geom_types[0] == 'LineString': + elif 'LineString' in geom_types: return MultiLineString(geoms) - elif geom_types[0] == 'Polygon': + elif 'Polygon' in geom_types: return MultiPolygon(geoms) - elif geom_types[0] == 'LinearRing': + elif 'LinearRing' in geom_types: return GeometryCollection(geoms) def from_element(self, element): From 8272b15efdb45b8f8bad3cdb22d011d3a016d9ee Mon Sep 17 00:00:00 2001 From: Evan Derickson <3587185+dericke@users.noreply.github.com> Date: Sun, 29 Nov 2020 14:01:17 -0800 Subject: [PATCH 2/5] Undo comment change --- fastkml/geometry.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fastkml/geometry.py b/fastkml/geometry.py index e8d2d7bc..df6ecdad 100644 --- a/fastkml/geometry.py +++ b/fastkml/geometry.py @@ -164,8 +164,8 @@ def _etree_coordinates(self, coordinates): tuples = ('%f,%f' % tuple(c) for c in coordinates) elif len(coordinates[0]) == 3: # if clampToGround: - # if the altitude is ignored anyway, we may as well - # ignore the z-value + # if the altitude is ignored anyway, we may as well + # ignore the z-value # tuples = ('%f,%f' % tuple(c[:2]) for c in coordinates) # else: tuples = ('%f,%f,%f' % tuple(c) for c in coordinates) From 17db268cf7417124d7c107a5f5c1d01233193738 Mon Sep 17 00:00:00 2001 From: Evan Derickson <3587185+dericke@users.noreply.github.com> Date: Sun, 29 Nov 2020 14:34:09 -0800 Subject: [PATCH 3/5] Sourcery refactors --- fastkml/atom.py | 66 ++++++++++------------- fastkml/base.py | 10 +--- fastkml/geometry.py | 7 ++- fastkml/kml.py | 127 ++++++++++++++++++++------------------------ fastkml/styles.py | 22 ++++---- 5 files changed, 101 insertions(+), 131 deletions(-) diff --git a/fastkml/atom.py b/fastkml/atom.py index 42b78bc1..0f3e3704 100644 --- a/fastkml/atom.py +++ b/fastkml/atom.py @@ -87,10 +87,7 @@ def __init__( self, ns=None, href=None, rel=None, type=None, hreflang=None, title=None, length=None ): - if ns is None: - self.ns = NS - else: - self.ns = ns + self.ns = NS if ns is None else ns self.href = href self.rel = rel self.type = type @@ -104,22 +101,21 @@ def from_string(self, xml_string): def from_element(self, element): if self.ns + self.__name__.lower() != element.tag: raise TypeError + if element.get('href'): + self.href = element.get('href') else: - if element.get('href'): - self.href = element.get('href') - else: - logger.critical('required attribute href missing') - raise TypeError - if element.get('rel'): - self.rel = element.get('rel') - if element.get('type'): - self.type = element.get('type') - if element.get('hreflang'): - self.hreflang = element.get('hreflang') - if element.get('title'): - self.title = element.get('title') - if element.get('length'): - self.length = element.get('length') + logger.critical('required attribute href missing') + raise TypeError + if element.get('rel'): + self.rel = element.get('rel') + if element.get('type'): + self.type = element.get('type') + if element.get('hreflang'): + self.hreflang = element.get('hreflang') + if element.get('title'): + self.title = element.get('title') + if element.get('length'): + self.length = element.get('length') def etree_element(self): element = etree.Element(self.ns + self.__name__.lower()) @@ -171,10 +167,7 @@ class _Person(object): # contains an email address for the person. def __init__(self, ns=None, name=None, uri=None, email=None): - if ns is None: - self.ns = NS - else: - self.ns = ns + self.ns = NS if ns is None else ns self.name = name self.uri = uri self.email = email @@ -191,10 +184,9 @@ def etree_element(self): # XXX validate uri uri = etree.SubElement(element, "%suri" % self.ns) uri.text = self.uri - if self.email: - if check_email(self.email): - email = etree.SubElement(element, "%semail" % self.ns) - email.text = self.email + if self.email and check_email(self.email): + email = etree.SubElement(element, "%semail" % self.ns) + email.text = self.email return element def from_string(self, xml_string): @@ -203,17 +195,15 @@ def from_string(self, xml_string): def from_element(self, element): if self.ns + self.__name__.lower() != element.tag: raise TypeError - else: - name = element.find('%sname' % self.ns) - if name is not None: - self.name = name.text - uri = element.find('%suri' % self.ns) - if uri is not None: - self.uri = uri.text - email = element.find('%semail' % self.ns) - if email is not None: - if check_email(email.text): - self.email = email.text + name = element.find('%sname' % self.ns) + if name is not None: + self.name = name.text + uri = element.find('%suri' % self.ns) + if uri is not None: + self.uri = uri.text + email = element.find('%semail' % self.ns) + if email is not None and check_email(email.text): + self.email = email.text def to_string(self, prettyprint=True): """ Return the ATOM Object as serialized xml """ diff --git a/fastkml/base.py b/fastkml/base.py index 8cd39dc4..ccc061a4 100644 --- a/fastkml/base.py +++ b/fastkml/base.py @@ -28,10 +28,7 @@ class _XMLObject(object): ns = None def __init__(self, ns=None): - if ns is None: - self.ns = config.KMLNS - else: - self.ns = ns + self.ns = config.KMLNS if ns is None else ns def etree_element(self): if self.__name__: @@ -77,10 +74,7 @@ class _BaseObject(_XMLObject): def __init__(self, ns=None, id=None): super(_BaseObject, self).__init__(ns) self.id = id - if ns is None: - self.ns = config.KMLNS - else: - self.ns = ns + self.ns = config.KMLNS if ns is None else ns def etree_element(self): element = super(_BaseObject, self).etree_element() diff --git a/fastkml/geometry.py b/fastkml/geometry.py index df6ecdad..53b1de13 100644 --- a/fastkml/geometry.py +++ b/fastkml/geometry.py @@ -31,15 +31,15 @@ # to another geometry. I've seen some signs of this changing in GEOS, # but until it does I don't think there's any point to the class. # It wouldn't be much more than a list of geometries. - from pygeoif.geometry import GeometryCollection from shapely.geometry import asShape except ImportError: from pygeoif.geometry import Point, LineString, Polygon from pygeoif.geometry import MultiPoint, MultiLineString, MultiPolygon from pygeoif.geometry import LinearRing - from pygeoif.geometry import GeometryCollection from pygeoif.geometry import as_shape as asShape +from pygeoif.geometry import GeometryCollection + import re import fastkml.config as config @@ -322,11 +322,10 @@ def _get_coordinates(self, element): # spaces. Clean up badly formatted tuples by stripping # space following commas. latlons = re.sub(r', +', ',', coordinates.text.strip()).split() - coords = [ + return [ [float(c) for c in latlon.split(',')] for latlon in latlons ] - return coords def _get_linear_ring(self, element): # LinearRing in polygon diff --git a/fastkml/kml.py b/fastkml/kml.py index 22321e82..529896e6 100644 --- a/fastkml/kml.py +++ b/fastkml/kml.py @@ -78,10 +78,7 @@ def __init__(self, ns=None): """ self._features = [] - if ns is None: - self.ns = config.KMLNS - else: - self.ns = ns + self.ns = config.KMLNS if ns is None else ns def from_string(self, xml_string): """ create a KML object from a xml string""" @@ -93,26 +90,26 @@ def from_string(self, xml_string): else: element = etree.XML(xml_string) - if element.tag.endswith('kml'): - ns = element.tag.rstrip('kml') - documents = element.findall('%sDocument' % ns) - for document in documents: - feature = Document(ns) - feature.from_element(document) - self.append(feature) - folders = element.findall('%sFolder' % ns) - for folder in folders: - feature = Folder(ns) - feature.from_element(folder) - self.append(feature) - placemarks = element.findall('%sPlacemark' % ns) - for placemark in placemarks: - feature = Placemark(ns) - feature.from_element(placemark) - self.append(feature) - else: + if not element.tag.endswith('kml'): raise TypeError + ns = element.tag.rstrip('kml') + documents = element.findall('%sDocument' % ns) + for document in documents: + feature = Document(ns) + feature.from_element(document) + self.append(feature) + folders = element.findall('%sFolder' % ns) + for folder in folders: + feature = Folder(ns) + feature.from_element(folder) + self.append(feature) + placemarks = element.findall('%sPlacemark' % ns) + for placemark in placemarks: + feature = Placemark(ns) + feature.from_element(placemark) + self.append(feature) + def etree_element(self): # self.ns may be empty, which leads to unprefixed kml elements. # However, in this case the xlmns should still be mentioned on the kml @@ -309,10 +306,7 @@ def timeStamp(self): @timeStamp.setter def timeStamp(self, dt): - if dt is None: - self._time_stamp = None - else: - self._time_stamp = TimeStamp(timestamp=dt) + self._time_stamp = None if dt is None else TimeStamp(timestamp=dt) if self._time_span is not None: logger.warn('Setting a TimeStamp, TimeSpan deleted') self._time_span = None @@ -404,24 +398,25 @@ def styles(self): @property def snippet(self): - if self._snippet: - if isinstance(self._snippet, dict): - text = self._snippet.get('text') - if text: - assert (isinstance(text, basestring)) - max_lines = self._snippet.get('maxLines', None) - if max_lines is None: - return {'text': text} - elif int(max_lines) > 0: - # if maxLines <=0 ignore it - return {'text': text, 'maxLines': max_lines} - elif isinstance(self._snippet, basestring): - return self._snippet - else: - raise ValueError( - "Snippet must be dict of " - "{'text':t, 'maxLines':i} or string" - ) + if not self._snippet: + return + if isinstance(self._snippet, dict): + text = self._snippet.get('text') + if text: + assert (isinstance(text, basestring)) + max_lines = self._snippet.get('maxLines', None) + if max_lines is None: + return {'text': text} + elif int(max_lines) > 0: + # if maxLines <=0 ignore it + return {'text': text, 'maxLines': max_lines} + elif isinstance(self._snippet, basestring): + return self._snippet + else: + raise ValueError( + "Snippet must be dict of " + "{'text':t, 'maxLines':i} or string" + ) @snippet.setter def snippet(self, snip=None): @@ -526,16 +521,10 @@ def from_element(self, element): self.description = description.text visibility = element.find('%svisibility' % self.ns) if visibility is not None: - if visibility.text in ['1', 'true']: - self.visibility = 1 - else: - self.visibility = 0 + self.visibility = 1 if visibility.text in ['1', 'true'] else 0 isopen = element.find('%sopen' % self.ns) if isopen is not None: - if isopen.text in ['1', 'true']: - self.isopen = 1 - else: - self.isopen = 0 + self.isopen = 1 if isopen.text in ['1', 'true'] else 0 styles = element.findall('%sStyle' % self.ns) for style in styles: s = Style(self.ns) @@ -830,7 +819,7 @@ def altitudeMode(self): @altitudeMode.setter def altitudeMode(self, mode): if mode in ('clampToGround', 'absolute'): - self._altitudeMode = str(mode) + self._altitudeMode = str(mode) else: self._altitudeMode = 'clampToGround' @@ -971,8 +960,7 @@ class Document(_Container): def schemata(self): if self._schemata: - for schema in self._schemata: - yield schema + yield from self._schemata def append_schema(self, schema): if self._schemata is None: @@ -1161,7 +1149,7 @@ def parse_str(self, datestr): year = int(datestr.split('-')[0]) month = int(datestr.split('-')[1]) dt = datetime(year, month, day) - elif len(datestr) == 8 or len(datestr) == 10: + elif len(datestr) in [8, 10]: resolution = 'date' dt = dateutil.parser.parse(datestr) elif len(datestr) > 10: @@ -1280,15 +1268,15 @@ def __init__(self, ns=None, id=None, name=None, fields=None): @property def simple_fields(self): - sfs = [] - for simple_field in self._simple_fields: - if simple_field.get('type') and simple_field.get('name'): - sfs.append({ - 'type': simple_field['type'], - 'name': simple_field['name'], - 'displayName': simple_field.get('displayName') - }) - return tuple(sfs) + return tuple( + { + 'type': simple_field['type'], + 'name': simple_field['name'], + 'displayName': simple_field.get('displayName'), + } + for simple_field in self._simple_fields + if simple_field.get('type') and simple_field.get('name') + ) @simple_fields.setter def simple_fields(self, fields): @@ -1352,10 +1340,11 @@ def from_element(self, element): sfname = simple_field.get('name') sftype = simple_field.get('type') display_name = simple_field.find('%sdisplayName' % self.ns) - if display_name is not None: - sfdisplay_name = display_name.text - else: - sfdisplay_name = None + sfdisplay_name = ( + display_name.text + if display_name is not None + else None + ) self.append(sftype, sfname, sfdisplay_name) def etree_element(self): diff --git a/fastkml/styles.py b/fastkml/styles.py index 0beb9afe..3e0a38a3 100644 --- a/fastkml/styles.py +++ b/fastkml/styles.py @@ -179,18 +179,16 @@ def from_element(self, element): def etree_element(self): element = super(StyleMap, self).etree_element() - if self.normal: - if isinstance(self.normal, (Style, StyleUrl)): - pair = etree.SubElement(element, "%sPair" % self.ns) - key = etree.SubElement(pair, "%skey" % self.ns) - key.text = 'normal' - pair.append(self.normal.etree_element()) - if self.highlight: - if isinstance(self.highlight, (Style, StyleUrl)): - pair = etree.SubElement(element, "%sPair" % self.ns) - key = etree.SubElement(pair, "%skey" % self.ns) - key.text = 'highlight' - pair.append(self.highlight.etree_element()) + if self.normal and isinstance(self.normal, (Style, StyleUrl)): + pair = etree.SubElement(element, "%sPair" % self.ns) + key = etree.SubElement(pair, "%skey" % self.ns) + key.text = 'normal' + pair.append(self.normal.etree_element()) + if self.highlight and isinstance(self.highlight, (Style, StyleUrl)): + pair = etree.SubElement(element, "%sPair" % self.ns) + key = etree.SubElement(pair, "%skey" % self.ns) + key.text = 'highlight' + pair.append(self.highlight.etree_element()) return element From 122952a98d78ac2dc95bf5c0d0691ddacfcac706 Mon Sep 17 00:00:00 2001 From: Evan Derickson <3587185+dericke@users.noreply.github.com> Date: Sun, 29 Nov 2020 14:49:08 -0800 Subject: [PATCH 4/5] Fix list comprehension indentation --- fastkml/geometry.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fastkml/geometry.py b/fastkml/geometry.py index 53b1de13..796eed14 100644 --- a/fastkml/geometry.py +++ b/fastkml/geometry.py @@ -381,7 +381,7 @@ def _get_multigeometry(self, element): '%sinnerBoundaryIs' % self.ns ) ibs = [self._get_linear_ring(inner_boundary) - for inner_boundary in inner_boundaries] + for inner_boundary in inner_boundaries] geoms.append(Polygon(ob, ibs)) linearings = element.findall('%sLinearRing' % self.ns) if linearings: From da750f9fa0d26c7d8b297ba5c82a7e4a894de75c Mon Sep 17 00:00:00 2001 From: Evan Derickson <3587185+dericke@users.noreply.github.com> Date: Sun, 29 Nov 2020 14:58:03 -0800 Subject: [PATCH 5/5] Moving stdlib import above contrib imports --- fastkml/geometry.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fastkml/geometry.py b/fastkml/geometry.py index 796eed14..e8aeba9a 100644 --- a/fastkml/geometry.py +++ b/fastkml/geometry.py @@ -19,6 +19,8 @@ Import the geometries from shapely if it is installed or otherwise from Pygeoif """ +import re + try: from shapely.geometry import Point, LineString, Polygon from shapely.geometry import MultiPoint, MultiLineString, MultiPolygon @@ -40,8 +42,6 @@ from pygeoif.geometry import as_shape as asShape from pygeoif.geometry import GeometryCollection - -import re import fastkml.config as config from .config import etree