BUG: Queen and Rook from_dataframe do not match docs

libpysal/weights/contiguity.py

@@ -20,6 +20,47 @@
 
 __all__ = ['Rook', 'Queen', 'Voronoi']
 
+
+def _rq_from_data_frame(cls, df, geom_col='geometry',
+                        idVariable=None, ids=None, id_order=None,
+                        **kwargs):
+    """Helper function to handle setting rook/queen ids from a dataframe
+
+
+        Parameters
+        ---------
+        cls         : class
+                      class that will nest this within a classmethod
+        df          : DataFrame
+                      a :class: `pandas.DataFrame` containing geometries to use
+                      for spatial weights
+        geom_col    : string
+                      the name of the column in `df` that contains the
+                      geometries. Defaults to `geometry`
+        idVariable  : string
+                      the name of the column to use as IDs. If nothing is
+                      provided, the dataframe index is used
+        ids         : list
+                      a list of ids to use to index the spatial weights object.
+                      Order is not respected from this list.
+        id_order    : list
+                      an ordered list of ids to use to index the spatial weights
+                      object. If used, the resulting weights object will iterate
+                      over results in the order of the names provided in this
+                      argument. 
+    """
+
+
+    if idVariable is not None:
+        ids = df.get(idVariable).tolist() #  idVariable takes precedent 
+        if id_order is None:
+            id_order = ids
+    else:
+        ids = df.index.tolist()
+        id_order = ids
+    return cls.from_iterable(df[geom_col].tolist(), ids=ids,
+                                id_order=id_order, **kwargs)
+
 class Rook(W):
     """
     Construct a weights object from a collection of pysal polygons that share at least one edge.
@@ -161,25 +202,14 @@ def from_dataframe(cls, df, geom_col='geometry',
         :class:`libpysal.weights.weights.W`
         :class:`libpysal.weights.contiguity.Rook`
         """
-        if id_order is not None:
-            if id_order is True and ((idVariable is not None) 
-                                     or (ids is not None)):
-                # if idVariable is None, we want ids. Otherwise, we want the
-                # idVariable column
-                id_order = list(df.get(idVariable, ids))
-            else:
-                id_order = df.get(id_order, ids)
-        elif idVariable is not None:
-            ids = df.get(idVariable).tolist()
-        elif isinstance(ids, str):
-            ids = df.get(ids).tolist()
-        return cls.from_iterable(df[geom_col].tolist(), ids=ids,
-                                 id_order=id_order, **kwargs)
+        return _rq_from_data_frame(cls, df, geom_col=geom_col,
+                                   idVariable=idVariable, ids=ids,
+                                   id_order=id_order, **kwargs)
 
 class Queen(W):
     """
     Construct a weights object from a collection of pysal polygons that share at least one vertex.
-
+ 
     Parameters
     ----------
     polygons    : list
@@ -288,7 +318,8 @@ def from_iterable(cls, iterable, sparse=False, **kwargs):
         return w
 
     @classmethod
-    def from_dataframe(cls, df, geom_col='geometry', **kwargs):
+    def from_dataframe(cls, df, geom_col='geometry',
+                       idVariable=None, ids=None, id_order=None, **kwargs):
         """
         Construct a weights object from a pandas dataframe with a geometry
         column. This will cast the polygons to PySAL polygons, then build the W
@@ -319,25 +350,10 @@ def from_dataframe(cls, df, geom_col='geometry', **kwargs):
         :class:`libpysal.weights.weights.W`
         :class:`libpysal.weights.contiguity.Queen`
         """
-        idVariable = kwargs.pop('idVariable', None)
-        ids = kwargs.pop('ids', None)
-        id_order = kwargs.pop('id_order', None)
-        if id_order is not None:
-            if id_order is True and ((idVariable is not None) 
-                                     or (ids is not None)):
-                # if idVariable is None, we want ids. Otherwise, we want the
-                # idVariable column
-                ids = list(df.get(idVariable, ids))
-                id_order = ids
-            elif isinstance(id_order, str):
-                ids = df.get(id_order, ids)
-                id_order = ids
-        elif idVariable is not None:
-            ids = df.get(idVariable).tolist()
-        elif isinstance(ids, str):
-            ids = df.get(ids).tolist()
-        w = cls.from_iterable(df[geom_col].tolist(), ids=ids, id_order=id_order, **kwargs)
-        return w
+        return _rq_from_data_frame(cls, df, geom_col=geom_col,
+                                   idVariable=idVariable, ids=ids,
+                                   id_order=id_order, **kwargs)
+
 
 def Voronoi(points, criterion='rook', clip='ahull', **kwargs):
     """

libpysal/weights/tests/test_contiguity.py

@@ -122,6 +122,27 @@ def test_linestrings(self):
         computed = self.cls.from_dataframe(eberly).sparse.toarray()
         np.testing.assert_array_equal(eberly_w, computed)
 
+    @ut.skipIf(GEOPANDAS_EXTINCT, 'Missing Geopandas')
+    def test_from_dataframe(self):
+        p = ("https://github.com/rsbivand/ectqg19-workshop/"\
+            "raw/master/data/lux_regions.gpkg")
+        db = geopandas.read_file(p)
+        wq = self.cls.from_dataframe(db)
+        self.assertEqual(wq.id_order[:5], [0, 1, 2, 3, 4])
+        wq_idv = self.cls.from_dataframe(db, idVariable='LAU2')
+        ido = ['0214', '1002', '0204', '0310', '0409']
+        self.assertEqual(wq_idv.id_order[:5], ido)
+        wq_sind = self.cls.from_dataframe(db.set_index('LAU2'))
+        self.assertEqual(wq_sind.id_order[:5], ido)
+        n_0 = [41, 2, 47, 7]
+        n_s = db.LAU2[n_0]
+        self.assertEqual(set(n_s) ,set(wq_idv.neighbors['0214']))
+        self.assertEqual(wq_sind.neighbors, wq_idv.neighbors)
+
+
+
+
+
 class Test_Rook(ut.TestCase, Contiguity_Mixin):
     def setUp(self):
         Contiguity_Mixin.setUp(self)
@@ -144,6 +165,9 @@ def test_voronoiW(self):
                                         1: [2], 2: [0, 1, 4],
                                         3: [0, 4], 4: [0, 2, 3]})
 
+
+
+
 q = ut.TestLoader().loadTestsFromTestCase(Test_Queen)
 r = ut.TestLoader().loadTestsFromTestCase(Test_Rook)
 suite = ut.TestSuite([q, r])