Add spatial-join algorithm #475

mrocklin · 2017-08-05T00:50:33Z

This uses GEOS's STRTree to implement a spatial join.

mrocklin · 2017-08-05T00:54:26Z

I get 10-15x speedups with this over the previous implementation.

mrocklin · 2017-08-05T01:15:37Z

Along with #474 we're now at 32 failed tests (mostly because we just replaced/deleted all of the previously failing spatial join tests)

===== 32 failed, 255 passed, 5 skipped, 6 xfailed, 12 warnings in 72.11 seconds =====

jorisvandenbossche · 2017-08-07T09:03:22Z

Just to know the status of this: does the new algorithm pass the existing tests? Or is there some change in behaviour?

jorisvandenbossche · 2017-08-07T09:05:55Z

Is it possible you forgot to include the actual algos.c file?

mrocklin · 2017-08-07T17:50:13Z

Is it possible you forgot to include the actual algos.c file?

Thanks added

Just to know the status of this: does the new algorithm pass the existing tests? Or is there some change in behaviour?

The existing tests themselves rely on pandas components that don't function well. We do support all of the options that the old implementation supported but I wouldn't be surprised if there were changes around column ordering or similar issues.

At least in my basic experiments this implementation has performed well

mrocklin · 2017-08-16T15:03:46Z

@jorisvandenbossche I'm inclined to merge this into the geopandas-cython branch. Any thoughts or objections?

jorisvandenbossche · 2017-08-16T16:35:19Z

Taking a more detailed look now.

jorisvandenbossche · 2017-08-16T17:09:49Z

Checked out the branch, and while trying to get my original examples/tests working, I am running into a strange issue:

First it works, but after restarting the ipython session (without doing anything else in the meantime), it fails returning empty geometry objects (in fact I was trying things out in a notebook and got empty polygons, and therefore made a reproducible example, did a clean rebuild of the branch, and then got the following running this example in a terminal session)

(dev) joris@joris-XPS-13-9350:~/scipy$ ipython
Python 3.5.2 |Continuum Analytics, Inc.| (default, Jul  2 2016, 17:53:06) 
Type 'copyright', 'credits' or 'license' for more information
IPython 6.0.0 -- An enhanced Interactive Python. Type '?' for help.

In [1]: from geopandas import GeoSeries, GeoDataFrame, sjoin
   ...: from shapely.geometry import Polygon

In [2]: polys1 = GeoSeries(
   ...:     [Polygon([(0, 0), (5, 0), (5, 5), (0, 5)]),
   ...:      Polygon([(5, 5), (6, 5), (6, 6), (5, 6)]),
   ...:      Polygon([(6, 0), (9, 0), (9, 3), (6, 3)])])
   ...: 
   ...: polys2 = GeoSeries(
   ...:     [Polygon([(1, 1), (4, 1), (4, 4), (1, 4)]),
   ...:      Polygon([(4, 4), (7, 4), (7, 7), (4, 7)]),
   ...:      Polygon([(7, 7), (10, 7), (10, 10), (7, 10)])])
   ...: 
   ...: df1 = GeoDataFrame({'geometry': polys1, 'df1': [0, 1, 2]})
   ...: df2 = GeoDataFrame({'geometry': polys2, 'df2': [3, 4, 5]})

In [3]: sjoin(df1, df2, 'intersects')
Out[3]: I am densified (3 elements)

   df1  df2  right_index                             geometry
0    0    4            1  POLYGON ((0 0, 5 0, 5 5, 0 5, 0 0))
0    0    3            0  POLYGON ((0 0, 5 0, 5 5, 0 5, 0 0))
1    1    4            1  POLYGON ((5 5, 6 5, 6 6, 5 6, 5 5))

In [4]: quit()
(dev) joris@joris-XPS-13-9350:~/scipy$ ipython
Python 3.5.2 |Continuum Analytics, Inc.| (default, Jul  2 2016, 17:53:06) 
Type 'copyright', 'credits' or 'license' for more information
IPython 6.0.0 -- An enhanced Interactive Python. Type '?' for help.

In [1]: from geopandas import GeoSeries, GeoDataFrame, sjoin
   ...: from shapely.geometry import Polygon

In [2]: polys1 = GeoSeries(
   ...:     [Polygon([(0, 0), (5, 0), (5, 5), (0, 5)]),
   ...:      Polygon([(5, 5), (6, 5), (6, 6), (5, 6)]),
   ...:      Polygon([(6, 0), (9, 0), (9, 3), (6, 3)])])
   ...: 
   ...: polys2 = GeoSeries(
   ...:     [Polygon([(1, 1), (4, 1), (4, 4), (1, 4)]),
   ...:      Polygon([(4, 4), (7, 4), (7, 7), (4, 7)]),
   ...:      Polygon([(7, 7), (10, 7), (10, 10), (7, 10)])])
   ...: 
   ...: df1 = GeoDataFrame({'geometry': polys1, 'df1': [0, 1, 2]})
   ...: df2 = GeoDataFrame({'geometry': polys2, 'df2': [3, 4, 5]})

In [3]: sjoin(df1, df2, 'intersects')
Out[3]: I am densified (3 elements)

   df1  df2       geometry
0    0    4  POLYGON EMPTY
0    0    3  POLYGON EMPTY
1    1    4  POLYGON EMPTY

Trying to debug it in the failing case, and all goes fine inside sjoin until the final GeoDataFrame construction.

jorisvandenbossche · 2017-08-16T17:15:55Z

Seems to boil down to a problem with passing a custom ordering in the GeoDataFrame constructor (which is also happening in the upstream geopandas-cython branch):

In [1]: from geopandas import GeoSeries, GeoDataFrame, sjoin
   ...: from shapely.geometry import Polygon, Point
   ...: 

In [2]: GeoDataFrame({'a': [1, 2, 3],
   ...:               'geometry': GeoSeries([Point(1, 1), Point(2, 2), Point(3, 3)])},
   ...:            columns=['geometry', 'a'], index=pd.Index([0, 0, 1]), geometry='geometry')
   ...:            
Out[2]: I am densified (3 elements)

        geometry
0  POLYGON EMPTY
1  POLYGON EMPTY
2  POLYGON EMPTY

In [3]: GeoDataFrame({'a': [1, 2, 3],
   ...:               'geometry': GeoSeries([Point(1, 1), Point(2, 2), Point(3, 3)])},
   ...:            columns=['a', 'geometry'], index=pd.Index([0, 0, 1]), geometry='geometry')
   ...:            
Out[3]: I am densified (3 elements)

   a     geometry
0  1  POINT (1 1)
1  2  POINT (2 2)
2  3  POINT (3 3)

(only the order in column=[..] is different)

but I still don't understand why it was not always failing ..

mrocklin · 2017-08-18T16:54:21Z

I think I've resolved this in the latest commit

jorisvandenbossche

Mainly some comments/questions for just better understanding the code.

Further:

could you leave the old tests intact? (or at least the first class with some pre-made objects and results, not necessarily the one using nybb) I tested this branch on those examples (https://gist.github.com/jorisvandenbossche/751809b631f30579b9859e6bf75d2657), and the new implementation gives the same results, with this exception of the exact ordering of columns / indices.
more binary predicates are now supported? Can you update the sjoin docstring for that?

jorisvandenbossche · 2017-08-18T19:43:06Z

geopandas/algos.c

+    // double time_spent = 0, time_spent_1 = 0, time_spent_2 = 0;
+
+    size_t l, r;
+    size_vector out;


what is size_vector?

ah, I see this is defined in algos.h. Can you put a comment there what it is used for?

jorisvandenbossche · 2017-08-18T19:43:27Z

geopandas/algos.c

+} geom_vector;
+
+
+void strtree_query_callback(void *item, void *vec)


can you add some more comments/docstrings?

I've added some more docstrings. Please feel free to point out further areas where this can be improved.

jorisvandenbossche · 2017-08-18T19:48:26Z

geopandas/algos.c

+#include <time.h>
+#include <stdbool.h>
+
+typedef char (*GEOSPredicate)(GEOSContextHandle_t handle, const GEOSGeometry *left, const GEOSGeometry *right);


only the prepared one is used here?

Yes, that's correct. Should we remove the other?

if it is not used for anything else, I would say yes

jorisvandenbossche · 2017-08-18T19:52:09Z

geopandas/tests/test_vectorized.py

+
+
+@pytest.mark.skip
+def test_bench_sjoin():


maybe not keep this in the test file?

jorisvandenbossche · 2017-08-18T19:54:28Z

geopandas/vectorized.pyx

+    out = np.empty((sv.n // 2, 2), dtype=np.uintp)
+
+    with nogil:
+        for idx in range(0, sv.n // 2):


can you add here some comments as well?

Is doing it manually faster than taking step-2 slices?

Unfortunately sv.a doesn't support slicing. It isn't a numpy array

jorisvandenbossche · 2017-08-18T20:05:27Z

geopandas/tools/sjoin.py

+    for name, series in left.iteritems():
+        if name in right.columns:
+            name = lsuffix + '_' + name
+        series.index = index[:n_left]


why is this one needed?
Because index is already equal to series.index no?

ah, for how='right' this is not the case. But can't we put this one then in the if block below?

It seems not. This causes some of the tests to fail.

jorisvandenbossche · 2017-08-18T20:06:51Z

geopandas/tools/sjoin.py

+    if how == 'left':
+        missing = pd.Index(np.arange(len(left_df))).difference(pd.Index(left_indices))
+        if len(missing):
+            left_indices = np.concatenate([left_indices, missing])


should we sort the indices here?

Is this typical?

The reason that I wonder it, is because (IIUC) that now entries in the left without a match will all end at the bottom (in case of a left/inner join), while it could make sense to keep the original ordering of the left frame ?

Also, the ordering of entries in the right frame when there are duplicate matches (duplicated rows of the right frame) does not always match the original.

Eg using the frames from the tests

geopandas.vectorized.cysjoin(df1.geometry._geometry_array.data, df2.geometry._geometry_array.data, 'intersects')

gives (with the previous commit -> still 2D array)

array([[0, 1], [0, 0], [1, 1]], dtype=uint64)

while it could make sense to ensure it is like

array([[0, 0], [0, 1], [1, 1]], dtype=uint64)

But, that would need a nested sort, and will maybe give a performance impediment that is not worth it?

Do you have an opinion about this one?

I don't have thoughts about the values of sorting or not. I think that you probably have a better sense of both the performance costs and the usability expectations.

jorisvandenbossche · 2017-08-18T20:08:46Z

geopandas/tools/sjoin.py

+            name = lsuffix + '_' + name
+        series.index = index[:n_left]
+        if how == 'right':
+            new = series.iloc[:0].reindex(index[n:])


this is to create an empty series of the correct dtype?

jorisvandenbossche · 2017-08-18T20:12:00Z

geopandas/tools/sjoin.py

+    names = []
+    for name, series in left.iteritems():
+        if name in right.columns:
+            name = lsuffix + '_' + name


lsuffix + '_' + name -> name + '_' + lsuffix ?

jorisvandenbossche · 2017-08-18T20:14:30Z

geopandas/tools/sjoin.py

+        names.append(name)
+    for name, series in right.iteritems():
+        if name in left.columns:
+            name = rsuffix + '_' + name


same here (switch around)

Now using name_suffix pattern

jorisvandenbossche · 2017-08-18T21:13:26Z

BTW, that was maybe not clear, but I am really happy with this new implementation of sjoin! (the python part, once the indexers are obtained) is now much more understandable)

jorisvandenbossche · 2017-08-18T21:25:23Z

geopandas/tests/test_vectorized.py

+    'covers',
+    'crosses',
+    'disjoint',
+    # 'equals',


What is the reason that this is commented out (seems useful)? Because equals has no prepared version?

jorisvandenbossche · 2017-08-18T21:30:04Z

geopandas/tools/sjoin.py

-    allowed_hows = ['left', 'right', 'inner']
-    if how not in allowed_hows:
-        raise ValueError("`how` was \"%s\" but is expected to be in %s" % \
-            (how, allowed_hows))


Keep this check?

Keeping these checks

jorisvandenbossche · 2017-08-18T21:30:23Z

geopandas/tools/sjoin.py

-            (op, allowed_ops))
-
-    if left_df.crs != right_df.crs:
-        print('Warning: CRS does not match!')


keep this one as well?

mrocklin · 2017-08-19T14:05:35Z

Regarding the old tests, how do you feel about column ordering.

(Pdb) pp res
I am densified (3 elements)
   df1  df2  index_right                             geometry
0    0    4            1  POLYGON ((0 0, 5 0, 5 5, 0 5, 0 0))
0    0    3            0  POLYGON ((0 0, 5 0, 5 5, 0 5, 0 0))
1    1    4            1  POLYGON ((5 5, 6 5, 6 6, 5 6, 5 5))
(Pdb) pp exp
   df1                             geometry  index_right  df2
0    0  POLYGON ((0 0, 5 0, 5 5, 0 5, 0 0))            0    3
0    0  POLYGON ((0 0, 5 0, 5 5, 0 5, 0 0))            1    4
1    1  POLYGON ((5 5, 6 5, 6 6, 5 6, 5 5))            1    4

I'm inclined to put the geometry to one side, either left or right. The current implementation puts it in the middle.

jorisvandenbossche · 2017-08-19T14:09:40Z

Yes, I think putting it at a side is actually better than in the middle. Since we default on putting it at the right side when constructing a frame, maybe best to do that here as well.
The only advantage of putting it in the middle is that it is clearer which columns have been added by the join.

jorisvandenbossche · 2017-08-19T14:14:38Z

geopandas/tools/sjoin.py

-                  pd.DataFrame(result[result['match_bool']==1])
-                  .drop('match_bool', axis=1)
-                  )
+        print("Warning: CRS does not match")


probably we should better turn this into an actual UserWarning

mrocklin · 2017-08-19T14:38:37Z

I've added back the old tests. What do you recommend for sorting by the secondary index? (Also please feel free to push commits to this branch if you like).

jorisvandenbossche

Thanks for the clarifications! Based on that, posted a new round of questions (using myself as a noob trying to understand the code well, to make sure it is as clear as possible for the future)

jorisvandenbossche · 2017-08-19T14:23:40Z

geopandas/algos.c

+{
+    size_t n, m;
+    GEOSGeometry **a;
+} geom_vector;


what is this one used for? (I don't seem to find a usage of geom_vector)

jorisvandenbossche · 2017-08-19T14:26:09Z

geopandas/algos.c

+
+
+/* Spatial join of two arrays of geometries over the predicate
+ * This creates an index for the right side


This sentence I don't fully understand. Doesn't it create an index for both sides?

jorisvandenbossche · 2017-08-19T14:26:42Z

geopandas/algos.c

+
+/* Spatial join of two arrays of geometries over the predicate
+ * This creates an index for the right side
+ * Finds all polygons in both sides that intersect with each other


I think it is general for 'geometries' and not limited to 'polygons' ?

This intersection step is what is done by the STRTree ?

jorisvandenbossche · 2017-08-19T14:30:54Z

geopandas/kvec.h

@@ -0,0 +1,90 @@
+/* The MIT License


Can you put at the top of this file a short note about why this file is includes (what it provides (implementation of resizeable vector) and why we need it (used to store matching indices in the spatial join implementation in algos.c)

Added a small note to the top of kvec.h

jorisvandenbossche · 2017-08-19T14:31:34Z

geopandas/algos.c

+
+    size_t l, r;                    // indices for left and right sides
+    GEOSPreparedGeometry* prepared; // Temporary prepared geometry for right side
+    size_vector out;                // output array of matching indices


The reason that a resizable vector is used is because we do not know in advance how many matches there will be, is that correct? If so, can you put somewhere a small comment about it (eg in the explanation above of how the algorithm works)

(starting a bit more to understand the use of the size_vector, at least I think :-))

jorisvandenbossche · 2017-08-19T14:34:22Z

geopandas/algos.c

+        if (left[l] != NULL)
+        {
+            // begin_1 = clock();
+            GEOSSTRtree_query_r(handle, tree, left[l], strtree_query_callback, &vec);


this stores indices of all intersecting geoms of the right array that intersect with left[l] in temporary array vec, correct?

jorisvandenbossche · 2017-08-19T14:39:14Z

geopandas/tools/sjoin.py

-                  )
+        print("Warning: CRS does not match")
+
+    left_indices, right_indices = cysjoin(left_df.geometry._geometry_array.data,


left_df.geometry._geometry_array.data can in principle be shortened to left._geometry_array.data

jorisvandenbossche · 2017-08-19T15:02:18Z

Thanks for the updates!
I can take a look at the old tests if you want (probably sorting both before asserting is easiest for now to ignore column/row ordering -> but then we now this is for now the difference with the old sjoin)

mrocklin · 2017-08-19T15:16:53Z

I suspect that you are more adept at normalizing outputs on the pandas side. If you have the time to take a look I would appreciate it.

…

On Sat, Aug 19, 2017 at 11:02 AM, Joris Van den Bossche < ***@***.***> wrote: Thanks for the updates! I can take a look at the old tests if you want (probably sorting both before asserting is easiest for now to ignore column/row ordering -> but then we now this is for now the difference with the old sjoin) — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#475 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AASszMx3HOuPX9HJIf5JXR66hwUQlnS2ks5sZvj7gaJpZM4OuUNf> .

jorisvandenbossche · 2017-08-19T15:19:52Z

Yes, will do.
BTW, I am adding a benchmark suite (#497), and added there the example you had here before in the tests.
(but didn't compare yet the geopandas-cython branch with master)

jorisvandenbossche · 2017-08-19T21:54:10Z

So to get the old tests running (locally), I only had to sort the rows and change an error type (apart from the column ordering you already did?)

jorisvandenbossche · 2017-08-19T22:34:30Z

Just wanted to look at the travis test results, but apparently something goes wrong during compiling (so everything fails)
This was not the case for previous PRs / geopandas-cython branch. But I also don't directly see an indication what could be the cause in this PR.

mrocklin · 2017-08-20T00:00:21Z

Builds now work

jorisvandenbossche · 2017-08-20T00:06:27Z

Some vectorized tests for sjoin are failing (because the result is no longer an array, but tuple of arrays)

mrocklin · 2017-08-20T02:13:32Z

Some vectorized tests for sjoin are failing (because the result is no longer an array, but tuple of arrays)

Fixed

mrocklin · 2017-08-20T18:05:54Z

@jorisvandenbossche any objection to merging this in?

mrocklin · 2017-08-20T20:51:33Z

.gitignore

@@ -9,6 +9,7 @@ geopandas/version.py
 doc/_static/world_*
 examples/nybb_*.zip
 .cache/
+*.c


We don't want to ignore algos.c

But that is added, so then it is no problem?
I thought this just means that you have to force add new c files, but maybe that's an incorrect git understanding

the thing is that now vectorized.c is in the list of untracked file, I can of course also add geopandas/vectorized.c explicitly here in .gitignore

Verified that your understanding is correct. I retract my previous comment.

jorisvandenbossche · 2017-08-20T20:56:01Z

geopandas/tools/tests/test_sjoin_old.py

@@ -227,7 +225,6 @@ def test_sjoin_values(self):
        self.assertEquals(df.shape, (12,8))

    @unittest.skipIf(str(pd.__version__) < LooseVersion('0.19'), pandas_0_18_problem)
-    @pytest.mark.xfail


this one is still failing (so the only sjoin test that should fail), but with an interesting error somewhere in the pandas munging of the frames. So one I have to look at when improving the pandas interactions, therefore removed it for now

jorisvandenbossche · 2017-08-20T21:05:16Z

@mrocklin Fixed the merge conflict and merged. Thanks a lot for this!

I think of the next follow-up issues:

Implement for 'equals' as well? (needs some adjustments because has no prepared version) This seems useful to me to be able to join on equal shapes.
Think/decide on preserving row ordering (want to keep original row ordering or not vs perf penalty to sort)

Do you think of others?

jorisvandenbossche · 2017-08-20T21:20:56Z

BTW, I ran the sjoin benchmarks of this branch vs upstream/master:
(before and after are logically switched)

       before           after         ratio
     [dddf039c]       [baa1601c]
+           855ms            19.6s    22.91  sjoin.Bench.time_sjoin('within')
+           1.57s            19.6s    12.45  sjoin.Bench.time_sjoin('intersects')
+           1.62s            19.9s    12.24  sjoin.Bench.time_sjoin('contains')

So some nice improvements!

* add sjoin algorithm (c/cythonized) * Fix constructor logic when geometry and columns both provided

mrocklin mentioned this pull request Aug 5, 2017

Follow-up - Refactor cythonize geometry series operations #473

Closed

8 tasks

jorisvandenbossche mentioned this pull request Aug 8, 2017

Fix GeoDataFrame.merge / pd.merge to return valid GeoDataFrame #481

Closed

mrocklin added 4 commits August 18, 2017 12:44

add sjoin algorithm

9a0d102

remove .c from gitignore

d73e913

add missing algos.c

e288068

Fix constructor logic when geometry and columns both provided

7c0e0b3

mrocklin force-pushed the sjoin2 branch from bcf2a12 to 7c0e0b3 Compare August 18, 2017 16:54

jorisvandenbossche reviewed Aug 18, 2017

View reviewed changes

respond to feedback on spatial joins

8dfe27b

jorisvandenbossche reviewed Aug 19, 2017

View reviewed changes

add back old sjoin tests

c7229ba

jorisvandenbossche reviewed Aug 19, 2017

View reviewed changes

mrocklin added 2 commits August 19, 2017 10:50

fix index_right/left in tests

19791f7

add additional comments

ce2bf29

get old tests passing

ebd1aaa

jorisvandenbossche force-pushed the sjoin2 branch from bdb9c73 to ebd1aaa Compare August 19, 2017 21:53

move int declaration out of for loop

db7e14a

fix cysjoin tests

dc3a335

Remove unused code

c4b8986

minor clean-up

f21a9d6

mrocklin commented Aug 20, 2017

View reviewed changes

jorisvandenbossche reviewed Aug 20, 2017

View reviewed changes

Merge remote-tracking branch 'upstream/geopandas-cython' into pr/475

2124542

jorisvandenbossche merged commit 43546c4 into geopandas:geopandas-cython Aug 20, 2017

jorisvandenbossche pushed a commit to jorisvandenbossche/geopandas that referenced this pull request Jul 1, 2019

Cython refactor: Add spatial-join algorithm (geopandas#475)

58e9b84

* add sjoin algorithm (c/cythonized) * Fix constructor logic when geometry and columns both provided

jorisvandenbossche pushed a commit to jorisvandenbossche/geopandas that referenced this pull request Sep 18, 2019

Cython refactor: Add spatial-join algorithm (geopandas#475)

24e1124

* add sjoin algorithm (c/cythonized) * Fix constructor logic when geometry and columns both provided

jorisvandenbossche pushed a commit to jorisvandenbossche/geopandas that referenced this pull request Sep 30, 2019

Cython refactor: Add spatial-join algorithm (geopandas#475)

65f743c

* add sjoin algorithm (c/cythonized) * Fix constructor logic when geometry and columns both provided

jorisvandenbossche mentioned this pull request Oct 6, 2019

Port the spatial join algorithm from geopandas pygeos/pygeos#52

Closed

		} geom_vector;


		void strtree_query_callback(void item, void vec)



		/* Spatial join of two arrays of geometries over the predicate
		* This creates an index for the right side

Add spatial-join algorithm #475

Add spatial-join algorithm #475

Conversation

mrocklin commented Aug 5, 2017

mrocklin commented Aug 5, 2017

mrocklin commented Aug 5, 2017

jorisvandenbossche commented Aug 7, 2017

jorisvandenbossche commented Aug 7, 2017

mrocklin commented Aug 7, 2017

mrocklin commented Aug 16, 2017

jorisvandenbossche commented Aug 16, 2017

jorisvandenbossche commented Aug 16, 2017

jorisvandenbossche commented Aug 16, 2017

mrocklin commented Aug 18, 2017

jorisvandenbossche left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jorisvandenbossche commented Aug 18, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mrocklin commented Aug 19, 2017

jorisvandenbossche commented Aug 19, 2017

Choose a reason for hiding this comment

mrocklin commented Aug 19, 2017

jorisvandenbossche left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jorisvandenbossche commented Aug 19, 2017

mrocklin commented Aug 19, 2017 via email

jorisvandenbossche commented Aug 19, 2017

jorisvandenbossche commented Aug 19, 2017

jorisvandenbossche commented Aug 19, 2017

mrocklin commented Aug 20, 2017

jorisvandenbossche commented Aug 20, 2017

mrocklin commented Aug 20, 2017

mrocklin commented Aug 20, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jorisvandenbossche commented Aug 20, 2017

jorisvandenbossche commented Aug 20, 2017 • edited

jorisvandenbossche commented Aug 20, 2017 •

edited