Catalog: support indexing non-contiguous catalog

We can extract what we need by iterating over the catalog, giving the
user what he wants without him having to worry about contiguity.

include/lsst/afw/table/python/catalog.h

@@ -77,6 +77,20 @@ void declareCatalogOverloads(PyCatalog<Record> &cls) {
         std::ptrdiff_t b = self.upper_bound(upper, key) - self.begin();
         return py::slice(a, b, 1);
     });
+
+    // Not extracting afw::geom::Angle columns because the units would be ambiguous
+    if (!std::is_same<T, geom::Angle>::value) {
+        cls.def("_getitem_", [](Catalog const& self, Key<T> const& key) -> ndarray::Array<T const, 1, 0> {
+            // Extract a column from a potentially non-contiguous Catalog
+            ndarray::Array<T, 1, 0> out = ndarray::allocate(self.size());
+            auto outIter = out.begin();
+            auto inIter = self.begin();
+            for (; inIter != self.end(); ++inIter, ++outIter) {
+                *outIter = inIter->get(key);
+            }
+            return out;
+        });
+    }
 }
 
 /**

python/lsst/afw/table/base/baseContinued.py

@@ -23,6 +23,7 @@
 
 from future.utils import with_metaclass
 from past.types import basestring
+from builtins import zip
 
 import numpy as np
 
@@ -127,8 +128,22 @@ def __getitem__(self, key):
                 stop = len(self)
             return self.subset(start, stop, step)
         elif isinstance(key, np.ndarray):
-            return self.subset(key)
+            if key.dtype == bool:
+                return self.subset(key)
+            raise RuntimeError("Unsupported array type for indexing non-contiguous Catalog: %s" %
+                               (key.dtype,))
         elif isinstance(key, Key) or isinstance(key, basestring):
+            if not self.isContiguous():
+                if isinstance(key, basestring):
+                    key = self.schema[key].asKey()
+                array = self._getitem_(key)
+                # This array doesn't share memory with the Catalog, so don't let it be modified by
+                # the user who thinks that the Catalog itself is being modified.
+                # Just be aware that this array can only be passed down to C++ as an ndarray::Array<T const>
+                # instead of an ordinary ndarray::Array<T>. If pybind isn't letting it down into C++,
+                # you may have left off the 'const' in the definition.
+                array.flags.writeable = False
+                return array
             return self.columns[key]
         else:
             return self._getitem_(key)

tests/test_sourceTable.py

@@ -659,6 +659,26 @@ def testRecordRepr(self):
         for field in ('id: 50', 'coord_ra: nan', 'coord_dec: nan'):
             self.assertIn(field, string)
 
+    def testGetNonContiguous(self):
+        """Check that we can index on non-contiguous tables"""
+        # Make a non-contiguous catalog
+        nonContiguous = type(self.catalog)(self.catalog.table)
+        for rr in reversed(self.catalog):
+            nonContiguous.append(rr)
+        num = len(self.catalog)
+        # Check assumptions
+        self.assertFalse(nonContiguous.isContiguous())  # We managed to produce a non-contiguous catalog
+        self.assertEqual(len(set(self.catalog["id"])), num)  # ID values are unique
+        # Indexing with boolean array
+        select = np.zeros(num, dtype=bool)
+        select[1] = True
+        self.assertEqual(nonContiguous[np.flip(select, 0)]["id"], self.catalog[select]["id"])
+        # Extracting a column
+        column = "a_flux"
+        array = nonContiguous[column]
+        self.assertFloatsEqual(np.flip(array, 0), self.catalog[column])
+        with self.assertRaises(ValueError):
+            array[1] = 1.2345  # Should be immutable
 
 class MemoryTester(lsst.utils.tests.MemoryTestCase):
     pass