Issue #160

mapmaker/flatmap/__init__.py

@@ -288,18 +288,18 @@ def get_feature_by_geojson_id(self, geojson_id: int) -> Optional[Feature]:
     #=========================================================================
         return self.__features_by_geojson_id.get(geojson_id)
 
-    def new_feature(self, layer_id: str, geometry, properties, is_group=False) -> Feature:
-    #=====================================================================================
-        self.__last_geojson_id += 1
+    def new_feature(self, layer_id: str, geometry, properties, is_group=False) -> Optional[Feature]:
+    #===============================================================================================
         properties['layer'] = layer_id
         self.properties_store.update_properties(properties)   # Update from JSON properties file
+        if (id:=properties.get('id')) is not None and id in self.__features_with_id:
+            log.error('Duplicate feature id', id=id)
+            return None
+        self.__last_geojson_id += 1
         feature = Feature(self.__last_geojson_id, geometry, properties, is_group=is_group)
         self.__features_by_geojson_id[feature.geojson_id] = feature
         if feature.id:
-            if feature.id in self.__features_with_id:
-                pass
-            else:
-                self.__features_with_id[feature.id] = feature
+            self.__features_with_id[feature.id] = feature
         if self.map_kind == MAP_KIND.FUNCTIONAL:
             if (name := properties.get('name', '')) != '':
                 self.__features_with_name[f'{layer_id}/{name.replace(" ", "_")}'] = feature
@@ -335,14 +335,14 @@ def __add_proxy_feature(self, feature: Feature, feature_model: str, proxy_seq: i
         if 'Polygon' not in feature.geometry.geom_type:
             log.warning('Proxy feature must have a polygon shape', type='proxy', models=feature_model, feature=feature)
         elif self.__bottom_exported_layer is not None:
-            self.__bottom_exported_layer.add_feature(
-                self.new_feature(self.__bottom_exported_layer.id, proxy_dot(feature.geometry, proxy_seq), {   # type: ignore
-                    'id': f'proxy_{proxy_seq}_on_{feature.id}',
-                    'tile-layer': FEATURES_TILE_LAYER,
-                    'models': feature_model,
-                    'kind': 'proxy'
-                })
-            )
+            proxy_feature = self.new_feature(self.__bottom_exported_layer.id, proxy_dot(feature.geometry, proxy_seq), {   # type: ignore
+                'id': f'proxy_{proxy_seq}_on_{feature.id}',
+                'tile-layer': FEATURES_TILE_LAYER,
+                'models': feature_model,
+                'kind': 'proxy'
+            })
+            if proxy_feature is not None:
+                self.__bottom_exported_layer.add_feature(proxy_feature)
 
     def add_layer(self, layer: MapLayer):
     #====================================
@@ -412,6 +412,8 @@ def add_zoom_point(self, feature: Feature, description: Optional[str]=None) -> O
                 'kind': 'zoom-point',
                 'tile-layer': feature.properties['tile-layer']
             })
+            if zoom_point is None:
+                return
             if description is not None:
                 zoom_point.set_property('label', description)
             if (models := feature.models) is not None:
@@ -487,9 +489,11 @@ def __add_details(self):
             self.add_layer(layer)
 
 ## Put all this into 'features.py' as a function??
-    def __new_detail_feature(self, layer_id, detail_layer, minzoom, geometry, properties):
-    #=====================================================================================
+    def __new_detail_feature(self, layer_id, detail_layer, minzoom, geometry, properties) -> Optional[Feature]:
+    #==========================================================================================================
         new_feature = self.new_feature(layer_id, geometry, properties)
+        if new_feature is None:
+            return
         new_feature.set_property('minzoom', minzoom)
         if properties.get('type') == 'nerve':
             new_feature.set_property('type', 'nerve-section')
@@ -541,7 +545,7 @@ def __add_detail_features(self, layer, detail_layer, lowres_features):
                 new_feature = self.__new_detail_feature(layer.id, detail_layer, minzoom,
                                                         transform.transform_geometry(hires_feature.geometry),
                                                         hires_feature.properties)
-                if new_feature.has_property('details'):
+                if new_feature is not None and new_feature.has_property('details'):
                     extra_details.append(new_feature)
 
         # If hires features that we've just added also have details then add them

mapmaker/flatmap/layers.py

@@ -294,7 +294,7 @@ def add_group_features(self, group_name: str, features: list[Feature],
             'tile-layer': tile_layer
             }
 
-        layer_features = []    # Features that will be added to the layer
+        layer_features: list[Feature] = []    # Features that will be added to the layer
         grouped_properties = {
             'group': True,
             'interior': True,
@@ -305,8 +305,8 @@ def add_group_features(self, group_name: str, features: list[Feature],
         boundary_class = None
         boundary_lines = []
         boundary_polygon = None
-        dividers = []
-        regions = []
+        dividers: list[shapely.LineString] = []
+        regions: list[Feature] = []
 
         debug_group = False
         child_class = None
@@ -340,12 +340,17 @@ def add_group_features(self, group_name: str, features: list[Feature],
                 child_class = feature.pop_property('children')
                 grouped_properties.update(feature.properties)
             elif feature.get_property('region'):
-                regions.append(self.flatmap.new_feature(self.id, feature.geometry.representative_point(), feature.properties))
+                region_properties = feature.properties.copy()
+                # So that any region doesn't have a duplicate id
+                region_properties.pop('id', None)
+                region = self.flatmap.new_feature(self.id, feature.geometry.representative_point(), region_properties)
+                if region is not None:
+                    regions.append(region)
             elif feature.get_property('divider'):
                 if feature.geom_type == 'LineString':
-                    dividers.append(feature.geometry)
+                    dividers.append(feature.geometry)           # pyright: ignore[reportArgumentType]
                 elif feature.geom_type == 'Polygon':
-                    dividers.append(feature.geometry.boundary)
+                    dividers.append(feature.geometry.boundary)  # pyright: ignore[reportArgumentType]
                 if feature.visible():
                     layer_features.append(feature)
             elif not feature.get_property('interior'):
@@ -367,11 +372,9 @@ def add_group_features(self, group_name: str, features: list[Feature],
                 raise GroupValueError('{}: {}'.format(group_name, str(err)), features) from None
 
         if boundary_polygon is not None:
-            layer_features.append(
-                self.flatmap.new_feature(
-                    self.id,
-                    boundary_polygon,
-                    base_properties))
+            feature = self.flatmap.new_feature(self.id, boundary_polygon, base_properties)
+            if feature is not None:
+                layer_features.append(feature)
 
             if len(dividers):
                 # For all line dividers, if the end of a line is 'close to' another line
@@ -382,7 +385,7 @@ def add_group_features(self, group_name: str, features: list[Feature],
 
                 if debug_group:
                     save_geometry(shapely.MultiLineString(dividers), 'dividers.wkt')
-                dividers.append(boundary_polygon.boundary)
+                dividers.append(boundary_polygon.boundary)      # pyright: ignore[reportArgumentType]
 
                 divider_lines = connect_dividers(dividers, debug_group)
                 if debug_group:
@@ -398,11 +401,14 @@ def add_group_features(self, group_name: str, features: list[Feature],
                     if debug_group:
                         save_geometry(polygon, f'polygon_{n}.wkt')
                     prepared_polygon = shapely.prepared.prep(polygon)
-                    region_id = None
                     region_properties = base_properties.copy()
+                    # So that any region doesn't have a duplicate id
+                    region_properties.pop('id', None)
                     for region in filter(lambda p: prepared_polygon.contains(p.geometry), regions):
                         region_properties.update(region.properties)
-                        layer_features.append(self.flatmap.new_feature(self.id, polygon, region_properties))
+                        feature = self.flatmap.new_feature(self.id, polygon, region_properties)
+                        if feature is not None:
+                            layer_features.append(feature)
                         break
         else:
             for feature in features:
@@ -426,6 +432,8 @@ def add_group_features(self, group_name: str, features: list[Feature],
         # Construct a MultiPolygon containing all of the group's polygons
         # But only if the group contains a `.group` element...
 
+        # So that any grouped features don't have duplicate ids
+        grouped_properties.pop('id', None)
         feature_group = None  # Our returned Feature
         if generate_group:
             grouped_polygon_features = [ feature for feature in features if feature.is_group ]
@@ -443,9 +451,8 @@ def add_group_features(self, group_name: str, features: list[Feature],
                         self.id,
                         shapely.MultiPolygon(grouped_polygons).buffer(0),
                         grouped_properties, is_group=True)
-                layer_features.append(feature_group)
-                # So that any grouped lines don't have a duplicate id
-                grouped_properties.pop('id', None)
+                if feature_group is not None:
+                    layer_features.append(feature_group)
 
             grouped_lines = []
             for feature in grouped_polygon_features:
@@ -460,7 +467,8 @@ def add_group_features(self, group_name: str, features: list[Feature],
                       self.id,
                       shapely.MultiLineString(grouped_lines),
                       grouped_properties, is_group=True)
-                layer_features.append(feature_group)
+                if feature_group is not None:
+                    layer_features.append(feature_group)
 
         # Feature specific properties have precedence over group's
         default_properties = base_properties.copy()

mapmaker/properties/pathways.py

@@ -839,13 +839,12 @@ def __route_network_connectivity(self, network: Network):
                             properties['label'] = '\n'.join(labels)
                         elif path_model is not None:
                             properties['label'] = path.label
-                        if 'id' not in properties and path_model is not None:
-                            properties['id'] = path_model.replace(':', '_').replace('/', '_')
                         feature = self.__flatmap.new_feature('pathways', geometric_shape.geometry, properties)
-                        path_geojson_ids.append(feature.geojson_id)
-                        layer.add_feature(feature)
-                        if path_taxons is None:
-                            path_taxons = feature.get_property('taxons')
+                        if feature is not None:
+                            path_geojson_ids.append(feature.geojson_id)
+                            layer.add_feature(feature)
+                            if path_taxons is None:
+                                path_taxons = feature.get_property('taxons')
 
                 for geometric_shape in geometric_shapes:
                     properties = DEFAULT_PATH_PROPERTIES.copy() | added_properties
@@ -858,8 +857,9 @@ def __route_network_connectivity(self, network: Network):
                         if path_taxons is not None:
                             properties['taxons'] = path_taxons
                         feature = self.__flatmap.new_feature('pathways', geometric_shape.geometry, properties)
-                        path_geojson_ids.append(feature.geojson_id)
-                        layer.add_feature(feature)
+                        if feature is not None:
+                            path_geojson_ids.append(feature.geojson_id)
+                            layer.add_feature(feature)
 
                 nerve_feature_ids = routed_path.nerve_feature_ids
                 nerve_features = [self.__flatmap.get_feature(nerve_id) for nerve_id in nerve_feature_ids]
@@ -896,7 +896,8 @@ def __route_network_connectivity(self, network: Network):
                 nerve_polygon_feature = self.__flatmap.new_feature(
                     'pathways',
                     shapely.geometry.Polygon(feature.geometry.coords).buffer(0), properties)
-                layer.features.append(nerve_polygon_feature)
+                if nerve_polygon_feature is not None:
+                    layer.features.append(nerve_polygon_feature)
 
     def generate_connectivity(self, networks: Iterable[Network]):
     #============================================================

mapmaker/sources/mbfbioscience/__init__.py

@@ -146,12 +146,13 @@ def process(self):
             if anatomical_id is not None:
                 properties['models'] = anatomical_id
             feature = self.flatmap.new_feature(self.id, geometry, properties)
-            feature.set_property('dataset', self.__sparc_dataset)
-            feature.set_property('source', self.href)
-            self.__layer.add_feature(feature)
-            if anatomical_id == self.__boundary_id:
-                boundary_geometry = feature.geometry
-                self.__layer.boundary_feature = feature
+            if feature is not None:
+                feature.set_property('dataset', self.__sparc_dataset)
+                feature.set_property('source', self.href)
+                self.__layer.add_feature(feature)
+                if anatomical_id == self.__boundary_id:
+                    boundary_geometry = feature.geometry
+                    self.__layer.boundary_feature = feature
         if boundary_geometry is not None and boundary_geometry.geom_type == 'Polygon':
             # Save boundary in case transformed image is used for details
             self.__boundary_geometry = boundary_geometry

mapmaker/sources/powerpoint/__init__.py

@@ -128,9 +128,10 @@ def __process_shape_list(self, shapes: TreeList[Shape]) -> list[Feature]:
                     pass
                 elif not properties.get('exclude', False):
                     feature = self.flatmap.new_feature(self.id, shape.geometry, properties)
-                    features.append(feature)
-                    shape.set_property('geojson_id', feature.geojson_id)
-                    shape.set_property('feature', feature)
+                    if feature is not None:
+                        features.append(feature)
+                        shape.set_property('geojson_id', feature.geojson_id)
+                        shape.set_property('feature', feature)
         return features
 
 #===============================================================================

mapmaker/sources/svg/__init__.py

@@ -265,7 +265,9 @@ def __process_shape_list(self, shapes: TreeList[Shape], depth) -> list[Feature]:
             if isinstance(shape, TreeList):
                 self.__process_shape_list(shape, depth+1)
             elif not shape.properties.get('exclude', False):
-                features.append(self.flatmap.new_feature(self.id, shape.geometry, shape.properties))
+                feature = self.flatmap.new_feature(self.id, shape.geometry, shape.properties)
+                if feature is not None:
+                    features.append(feature)
         self.add_group_features(f'SVG_{depth}', features, outermost=(depth==0))
         return features