bug fixes and enhancements

Author: couteau

redistricting/gui/rdsdockwidget.py

@@ -60,8 +60,9 @@ def plan(self, value: RdsPlan):
                 self._plan.nameChanged.connect(self.planNameChanged)
                 self.lblPlanName.setText(self._plan.name)
 
-    def planNameChanged(self, name):
-        self.lblPlanName.setText(name)
+    def planNameChanged(self):
+        if self.sender() == self.plan:
+            self.lblPlanName.setText(self.plan.name)
 
     def btnHelpClicked(self):
         showHelp(self.helpContext)

redistricting/gui/wzpeditplangeofields.py

@@ -151,9 +151,9 @@ def fieldChanged(self, field):
             not self.cmbAddlGeoField.isExpression() or self.cmbAddlGeoField.isValidExpression()))
 
     def addField(self):
-        field, isExpression, isValid = self.cmbAddlGeoField.currentField()
+        field, _, isValid = self.cmbAddlGeoField.currentField()
         if not isValid:
             return
 
         layer = self.field('sourceLayer')
-        self.fieldsModel.appendField(layer, field, isExpression)
+        self.fieldsModel.appendField(layer, field)

redistricting/models/field.py

@@ -279,6 +279,9 @@ def getName(self, feature_or_key: Union[QgsFeature, str]) -> str:
 
     def makeJoin(self):
         rel = self.getRelation()
+        if rel is None:
+            return None
+
         pair = rel.fieldPairs()
         if len(pair) > 1:
             return None

redistricting/services/planbuilder.py

@@ -215,5 +215,7 @@ def createPlan(self, createLayers=True, planParent: Optional[QObject] = None) ->
 
         if createLayers:
             self.createLayers(plan)
+        elif self._geoPackagePath and self._geoPackagePath.exists():
+            plan.addLayersFromGeoPackage(self._geoPackagePath)
 
         return plan

redistricting/services/planeditor.py

@@ -25,7 +25,6 @@
 from typing import (
     Iterable,
     Optional,
-    Set,
     overload
 )
 
@@ -70,7 +69,7 @@ def __init__(self, parent: QObject = None, planUpdater: DistrictUpdater = None):
         self._modifiedFields = set()
 
     @property
-    def modifiedFields(self) -> Set[str]:
+    def modifiedFields(self) -> set[str]:
         return self._modifiedFields
 
     def setProgress(self, progress: float):
@@ -150,7 +149,7 @@ def _addFieldToLayer(self, layer: QgsVectorLayer, fieldOrFieldName, fieldType=No
     def _updatePopFields(self):
         if self._plan.distLayer:
             layer = self._plan.distLayer
-            addedFields: Set[RdsField] = set(self._popFields) - set(self._plan.popFields)
+            addedFields: list[RdsField] = [f for f in self._popFields if f not in self._plan.popFields]
             if addedFields:
                 self._addFieldToLayer(layer, [f.makeQgsField() for f in addedFields])
 
@@ -160,11 +159,11 @@ def _updateDataFields(self):
         if self._plan.distLayer:
             layer = self._plan.distLayer
 
-            addedFields: Set[RdsDataField] = set(self._dataFields) - set(self._plan.dataFields)
+            addedFields: list[RdsDataField] = [f for f in self._dataFields if f not in self._plan.dataFields]
             if addedFields:
                 self._addFieldToLayer(layer, [f.makeQgsField() for f in addedFields])
 
-            removedFields: Set[RdsDataField] = set(self._plan.dataFields) - set(self._dataFields)
+            removedFields: list[RdsDataField] = [f for f in self._plan.dataFields if f not in self._dataFields]
             if removedFields:
                 provider = layer.dataProvider()
                 for f in removedFields:
@@ -177,7 +176,7 @@ def _updateDataFields(self):
 
     def _updateGeoFields(self):
         def removeFields():
-            removedFields: Set[RdsField] = set(self._plan.geoFields) - set(self._geoFields)
+            removedFields: list[RdsField] = [f for f in self._plan.geoFields if f not in self._geoFields]
             if removedFields:
                 provider = self._assignLayer.dataProvider()
                 fields = self._assignLayer.fields()
@@ -217,7 +216,7 @@ def completed():
 
         saveFields = self._plan.geoFields
         layer = self._plan.assignLayer
-        addedFields: Set[RdsField] = set(self._geoFields) - set(self._plan.geoFields)
+        addedFields: list[RdsField] = [f for f in self._geoFields if f not in self._plan.geoFields]
         if addedFields:
             self._addFieldToLayer(layer, [f.makeQgsField() for f in addedFields])
 

redistricting/services/planlistmodel.py

@@ -136,7 +136,8 @@ def planListUpdated(self):
         self.beginResetModel()
         self.endResetModel()
 
-    def updatePlan(self, plan):
+    def updatePlan(self):
+        plan = self.sender()
         idx1 = self.indexFromPlan(plan)
         idx2 = self.createIndex(idx1.row(), self.columnCount() - 1)
         self.dataChanged.emit(idx1, idx2)

redistricting/services/tasks/updatedistricts.py

@@ -130,15 +130,17 @@ def calcCutEdges(self) -> Union[int, None]:
             # select count of unit pairs where
             #   1) assigned districts are different (also takes care of excluding unassigned units from count),
             #   2) combination is unique (count a,b but not b,a),
-            #   3) bounding boxes overlap (using spatial index to minimize more intensive adjacency checks)
+            #   3) bounding boxes touch or overlap (using spatial index to minimize more intensive adjacency checks)
             #   4) units are adjacent at more than a point
-            sql = "SELECT count(*) " \
-                "FROM assignments a JOIN assignments b " \
-                f"ON b.{self.distField} != a.{self.distField} AND b.{self.geoIdField} > a.{self.geoIdField} " \
-                "AND b.fid IN(SELECT id FROM rtree_assignments_geometry r " \
-                "WHERE r.minx < st_maxx(a.geometry) and r.maxx >= st_minx(a.geometry) " \
-                "AND r.miny < st_maxy(a.geometry) and r.maxy >= st_miny(a.geometry)) " \
-                "AND st_length(st_intersection(a.geometry, b.geometry)) > 0 "
+            sql = f""""SELECT count(*)
+                FROM assignments a JOIN assignments b
+                ON b.{self.distField} != a.{self.distField} AND b.{self.geoIdField} > a.{self.geoIdField}
+                AND b.fid IN (
+                    SELECT id FROM rtree_assignments_geometry r
+                    WHERE r.minx <= st_maxx(a.geometry) and r.maxx >= st_minx(a.geometry)
+                    AND r.miny <= st_maxy(a.geometry) and r.maxy >= st_miny(a.geometry)
+                )
+                AND st_relate(a.geometry, b.geometry, 'F***1****')"""
 
             c = db.execute(sql)
             return c.fetchone()[0]

redistricting/utils/layer.py

@@ -45,7 +45,6 @@
     QgsFeedback,
     QgsVectorLayer
 )
-from shapely import wkb
 
 from .. import CanceledError
 from .intl import tr
@@ -83,6 +82,17 @@ def iterateWithProgress(self, it: Iterator, total: int = 0):
                 self.checkCanceled()
             yield n
 
+    def split_provider_url(self):
+        uri_parts = self._layer.dataProvider().dataSourceUri().split('|')
+        if len(uri_parts) <= 1:
+            raise ValueError("Could not determine table name from URI")
+        database = uri_parts[0]
+        lexer = shlex.shlex(uri_parts[1])
+        lexer.whitespace_split = True
+        lexer.whitespace = '&'
+        params = dict(pair.split('=', 1) for pair in lexer)
+        return database, params
+
     def read_qgis(
         self,
         columns: Optional[list[str]] = None,
@@ -92,58 +102,64 @@ def read_qgis(
         filt: Optional[dict[str, Any]] = None
     ) -> Union[pd.DataFrame, gpd.GeoDataFrame]:
         def prog_attributes(f: QgsFeature):
-            nonlocal count
-            count += 1
-            if count % chunksize == 0:
-                self.checkCanceled()
-                self.updateProgress(fc, count)
             attrs = [f.attribute(i) for i in indices]
             if read_geometry:
                 attrs.append(f.geometry().asWkb().data())
             return attrs
 
         if not chunksize:
             chunksize = 1
-        fc = self._layer.featureCount()
-        count = 0
+
         fields = self._layer.fields()
+        req = QgsFeatureRequest()
+        if self._feedback:
+            req.setFeedback(self._feedback)
+
         if columns is None:
             columns = fields.names()
-            gen = (prog_attributes(f) for f in self._layer.getFeatures())
+            indices = range(len(columns))
         else:
             indices = [fields.lookupField(c) for c in columns]
             if any((i == -1 for i in indices)):
                 raise RuntimeError("Bad fields")
-            req = QgsFeatureRequest()
             req.setSubsetOfAttributes(indices)
-            if filt:
-                filt = {f: f"{f'{v}' if isinstance(v, str) else v}" for f, v in filt.items()}
-                expr = f"{'AND'.join(f'{f} = {v}' for f,v in filt.items())}"
-                req.setFilterExpression(expr)
-            gen = (prog_attributes(f) for f in self._layer.getFeatures(req))
+
+        if filt:
+            expr = f"{' AND '.join(f'({f} = {v!r})' for f, v in filt.items())}"
+            req.setFilterExpression(expr)
+
+        if order:
+            clause = QgsFeatureRequest.OrderByClause(order)
+            orderby = QgsFeatureRequest.OrderBy([clause])
+            req.setOrderBy(orderby)
 
         if read_geometry:
-            columns = [*columns, "geometry"]
-            df = pd.DataFrame(gen, columns=columns)
-            df['geometry'] = df['geometry'].apply(wkb.loads)
-            df = gpd.GeoDataFrame(df, geometry="geometry", crs=self._layer.crs().authid())
+            columns.append('geometry')
+            df = gpd.GeoDataFrame.from_features(self._layer.getFeatures(req), self._layer.crs().authid(), columns)
         else:
+            gen = (prog_attributes(f) for f in self._layer.getFeatures(req))
             df = pd.DataFrame(gen, columns=columns)
 
-        if order and order in df.columns:
-            df = df.sort_values(order).set_index(order)
-
         return df
 
     def gpd_read(
             self,
-            source,
+            source=None,
             fc: int = 0,
             chunksize: Optional[int] = None,
             filt: Optional[dict[str, Any]] = None,
             **kwargs
     ) -> gpd.GeoDataFrame:
         df: gpd.GeoDataFrame = None
+
+        if source is None:
+            source, params = self.split_provider_url()
+            if "layer" not in kwargs:
+                kwargs["layer"] = params["layername"]
+
+        if filt is not None:
+            kwargs["where"] = " AND ".join(f"({f} = {v!r})" for f, v in filt.items())
+
         if (fc or chunksize):
             if chunksize is None:
                 divisions = 10
@@ -166,10 +182,6 @@ def gpd_read(
                 )
             else:
                 df = gpd.read_file(source, **kwargs)
-
-            if filt:
-                for f, v in filt.items():
-                    df = df[df[f] == v]
         else:
             df = gpd.read_file(source, **kwargs)
             self.updateProgress(len(df), len(df))
@@ -184,7 +196,8 @@ def read_layer(
         order: Optional[str] = ...,
         filt: Optional[dict[str, Any]] = ...,
         read_geometry: Literal[False] = ...,
-        chunksize: int = ...
+        chunksize: int = ...,
+        **kwargs
     ) -> pd.DataFrame:
         ...
 
@@ -195,7 +208,8 @@ def read_layer(
         order: Optional[str] = ...,
         filt: Optional[dict[str, Any]] = ...,
         read_geometry: Literal[True] = ...,
-        chunksize: int = ...
+        chunksize: int = ...,
+        **kwargs
     ) -> gpd.GeoDataFrame:
         ...
 
@@ -205,7 +219,8 @@ def read_layer(
             order=None,
             filt=None,
             read_geometry=True,
-            chunksize=0
+            chunksize=0,
+            **kwargs
     ) -> Union[pd.DataFrame, gpd.GeoDataFrame]:
         def makeSqlQuery():
             nonlocal filt
@@ -216,7 +231,7 @@ def makeSqlQuery():
                 cols = ",".join(columns)
                 if read_geometry and (g := self.getGeometryColumn(self._layer)):
                     cols += f",{g}"
-            sql = f"SELECT {cols} from {self.getTableName(self._layer)}"
+            sql = f"SELECT {cols} FROM {self.getTableName(self._layer)}"
             if filt or self._layer.subsetString():
                 filters = []
                 if filt:
@@ -248,21 +263,15 @@ def makeSqlQuery():
 
         if self._layer.storageType() in ("GPKG", "OpenFileGDB"):
             if read_geometry:
-                uri_parts = self._layer.dataProvider().dataSourceUri().split('|')
-                if len(uri_parts) <= 1:
-                    raise ValueError("Could not determine table name from URI")
-                database = uri_parts[0]
-                lexer = shlex.shlex(uri_parts[1])
-                lexer.whitespace_split = True
-                lexer.whitespace = '&'
-                params = dict(pair.split('=', 1) for pair in lexer)
-                df = self.gpd_read(database, self._layer.featureCount(), chunksize,
-                                   layer=params['layername'], columns=columns)
+                database, params = self.split_provider_url()
+                df = self.gpd_read(database, self._layer.featureCount(), chunksize, filt,
+                                   layer=params['layername'], columns=columns, **kwargs)
                 if order:
                     df = df.set_index(order).sort_index()
             else:
                 with self._connectSqlOgrSqlite(self._layer.dataProvider()) as db:
-                    df = pd.read_sql(makeSqlQuery(), db, index_col=order, columns=columns, chunksize=chunksize)
+                    df = pd.read_sql(makeSqlQuery(), db, index_col=order,
+                                     columns=columns, chunksize=chunksize, **kwargs)
                 if isinstance(df, Iterator):
                     df = pd.concat(self.iterateWithProgress(df, total))
         elif self._layer.dataProvider().name() in ('spatialite', 'SQLite'):
@@ -272,7 +281,7 @@ def makeSqlQuery():
                     shlex.split(re.sub(r' \(\w+\)', '', self._layer.dataProvider().dataSourceUri(True)))
                 )
                 df = self.gpd_read(params['dbname'], self._layer.featureCount(),
-                                   chunksize, layer=params['table'], columns=columns)
+                                   chunksize, layer=params['table'], columns=columns, **kwargs)
                 if order:
                     df = df.set_index(order).sort_index()
             else:
@@ -288,15 +297,17 @@ def makeSqlQuery():
                         db,
                         self.getGeometryColumn(self._layer),
                         index_col=order,
-                        chunksize=chunksize
+                        chunksize=chunksize,
+                        **kwargs
                     )
                 else:
                     df = pd.read_sql(
                         makeSqlQuery(),
                         db,
                         index_col=order,
                         columns=columns,
-                        chunksize=chunksize
+                        chunksize=chunksize,
+                        **kwargs
                     )
 
             if isinstance(df, Iterator):
@@ -307,7 +318,8 @@ def makeSqlQuery():
                 self._layer.featureCount(),
                 columns=columns,
                 chunksize=chunksize,
-                read_geometry=read_geometry
+                read_geometry=read_geometry,
+                **kwargs
             )
             if order:
                 df = df.set_index(order).sort_index()
@@ -333,11 +345,12 @@ def makeSqlQuery():
                         delimiter=delimiter,
                         header=header,
                         usecols=usecols,
-                        chunksize=chunksize
+                        chunksize=chunksize,
+                        **kwargs
                     )
                     df = pd.concat(self.iterateWithProgress(reader.get_chunk(), total))
                 else:
-                    df = pd.read_csv(uri_parts.path, delimiter=delimiter, header=header, usecols=usecols)
+                    df = pd.read_csv(uri_parts.path, delimiter=delimiter, header=header, usecols=usecols, **kwargs)
                 if header is None:
                     if len(columns) == len(df.columns):
                         df.columns = columns

tests/unittests/utils/test_layerreader.py

@@ -0,0 +1,18 @@
+import functools
+import timeit
+
+import redistricting
+
+
+# pylint: disable=import-outside-toplevel
+class TestLayerReader:
+    def test_readqgis(self, block_layer):
+        reader = redistricting.utils.layer.LayerReader(block_layer)
+
+        t1 = timeit.timeit(reader.read_qgis, number=20)
+        print(t1)
+
+    def test_gpd_read(self, block_layer):
+        reader = redistricting.utils.layer.LayerReader(block_layer)
+        t2 = timeit.timeit(functools.partial(reader.gpd_read, chunksize=-1), number=20)
+        print(t2)