ENH/PERF: use mask in factorize for nullable dtypes #33064

jorisvandenbossche · 2020-03-27T10:06:19Z

This adds the option to use a mask in the HashTable.factorize (implementation itself is in HashTable._unique).

That allows eg IntegerArray to use its mask and avoid the need to convert to object dtype or float dtype (which also avoids a copy of the data), and gives a nice speed-up (so checking for na_value or nan, only checking the mask):

Small example (for easier testing purposes I just added factorize2 that uses the mask, vs existing factorize, but that is not meant to stay of course):

In [1]: a = np.random.randint(0, 10000, 100000) 
   ...: mask = np.zeros(len(a), dtype=bool)  
   ...: mask[np.random.randint(0, len(a), 1000)] = True 
   ...: arr = pd.arrays.IntegerArray(a, mask)   

In [2]: arr.factorize()
Out[2]: 
(array([   0,    1,    2, ..., 3726, 2673, 5358]), <IntegerArray>
 [2532, 8355, 1885, 2253, 8517, 5615, 3146,  386, 9183, 6497,
  ...
   794, 8600,  823, 1541, 4373, 1205, 9605, 4576,  443, 2070]
 Length: 10000, dtype: Int64)

# using mask gives the same result
In [3]: arr.factorize2()  
Out[3]: 
(array([   0,    1,    2, ..., 3726, 2673, 5358]), <IntegerArray>
 [2532, 8355, 1885, 2253, 8517, 5615, 3146,  386, 9183, 6497,
  ...
   794, 8600,  823, 1541, 4373, 1205, 9605, 4576,  443, 2070]
 Length: 10000, dtype: Int64)

# on master we actually convert to object dtype with NaNs, which is very slow
In [4]: %timeit arr.factorize() 
12.3 ms ± 849 µs per loop (mean ± std. dev. of 7 runs, 100 loops each)

# on this PR, for comparision, I changed this to convert to float dtype with NaNs
# (however this is not robust for large ints)
In [4]: %timeit arr.factorize()
2.83 ms ± 51.4 µs per loop (mean ± std. dev. of 7 runs, 100 loops each)

# on this PR using the mask is faster than both approaches above
In [5]: %timeit arr.factorize2() 
771 µs ± 13.4 µs per loop (mean ± std. dev. of 7 runs, 1000 loops each)

Since this is adding an extra if branch to factorize, this can impact existing cases that don't use the mask, but I didn't see any noticeable effect (with few times switching between branches and rebuilding):

In [1]: a = np.random.randint(0, 10000, 100000) 
   ...: mask = np.zeros(len(a), dtype=bool)  
   ...: mask[np.random.randint(0, len(a), 1000)] = True 
   ...:  
   ...: a2 = a.copy().astype(float) 
   ...: a2[mask] = np.nan 

# factorize on an integer array
In [4]: %timeit pd.factorize(a) 
769 µs ± 31.8 µs per loop (mean ± std. dev. of 7 runs, 1000 loops each)  <- PR
745 µs ± 39.2 µs per loop (mean ± std. dev. of 7 runs, 1000 loops each) <- master
779 µs ± 60.1 µs per loop (mean ± std. dev. of 7 runs, 1000 loops each)  <- PR
759 µs ± 50 µs per loop (mean ± std. dev. of 7 runs, 1000 loops each)  <- master

# factorize on a float array with nans
In [5]: %timeit pd.factorize(a2) 
2.12 ms ± 18.1 µs per loop (mean ± std. dev. of 7 runs, 100 loops each)  <- PR
2.2 ms ± 116 µs per loop (mean ± std. dev. of 7 runs, 100 loops each) <- master
2.2 ms ± 102 µs per loop (mean ± std. dev. of 7 runs, 100 loops each)  <- PR
2.13 ms ± 47.8 µs per loop (mean ± std. dev. of 7 runs, 100 loops each)  <- master

So for the int array there seems to be a small slowdown (as expected), although it is still within the +/- bounds. And I would say it is also small enough (around 2-3%) to be OK with this slowdown given the benefits of being able to use the mask for other cases.

jorisvandenbossche · 2020-03-27T10:09:53Z

Additional notes:

This does only implement the mask for the numeric hashtables for now, and not for object/string, since we don't yet have a use case for that (string dtype doesn't store a mak)
This only focuses on factorize, so this is not used in eg unique (which can probably also benefit from this, but want to keep the PR focused here for now)

jorisvandenbossche · 2020-03-27T10:13:32Z

pandas/core/arrays/integer.py

-        return self.to_numpy(na_value=np.nan), np.nan
+        return self.to_numpy(dtype=float, na_value=np.nan), np.nan
+
+    def factorize2(self, na_sentinel: int = -1) -> Tuple[np.ndarray, "ExtensionArray"]:


This is of course not meant to stay. I think we have two options:

Override the base class factorize here on IntegerArray to use the mask (so basically rename factorize2 to factorize)

Expand the _values_for_factorize spec to allow it to return a mask instead of na_value. The base class factorize then would need to choose the correct path depending on whether _values_for_factorize returned a boolean ndarray, or something else (scalar na_value)

Short term, the first is the easiest. But long term, I think the second would be nice to allow external EAs to more easily use this as well (avoiding the need they have to override the base class factorize as well). But this second option has some more implications (need to update everywhere it is used internally).

I also think that expanding _values_for_factorize is the way to go, but I worry about inferring the EA wants a masked based on na_value being a boolean ndarray. It's an edge case, but consider a nested / ragged array where each "scalar" element is an ndarray.

Could we instead (or also) have a class attribute like _masked_factorize = True indicating that they want to used the masked variant?

It's an edge case, but consider a nested / ragged array where each "scalar" element is an ndarray.

Even in such a case, I would expect the na_value not to be an array. But OK, it's certainly not the most robust way.

An alternative could also be to return 3 values (values, None, mask) in case of a mask, and then we can check the number of items returned.
Also not the cleanest solution, though. I just also don't really like the _masked_factorize attribute .. Certainly if we would start doing this in other places as well, and then might need other similar attributes. Unless we would go for a single _is_masked attribute and then you need to go all or nothing.

option 2 (return a mask) would be my preference, even if its a breaking change, much cleaner

I don't think we should simply break this (we would break other projects like GeoPandas), since it is relatively easily avoidable.

If we want to replace na_value in factorize with passing a mask (with a given na_value you can compute the mask in advance with values == na_value), we could deprecate the option of returning an na_value from _values_for_factorize, though.

jreback

code change looks good. idealy we can just use masks rather than na_value in internal factorize / unique. though that might be some work (maybe?). e.g. easy enough to construct a mask.

this PR? or followon?

jreback · 2020-03-27T16:34:23Z

pandas/core/arrays/integer.py

-        return self.to_numpy(na_value=np.nan), np.nan
+        return self.to_numpy(dtype=float, na_value=np.nan), np.nan
+
+    def factorize2(self, na_sentinel: int = -1) -> Tuple[np.ndarray, "ExtensionArray"]:


option 2 (return a mask) would be my preference, even if its a breaking change, much cleaner

jorisvandenbossche · 2020-03-27T16:39:42Z

idealy we can just use masks rather than na_value in internal factorize / unique. though that might be some work (maybe?). e.g. easy enough to construct a mask.

It would simplify the hashtable, but I am not sure it would always be beneficial performance-wise. I would need to do some testing to compare pre-computing the mask vs the current implementation to check na_value for dtypes that don't already have the mask available.

And we still have the val != val (in addition to specifying an na_value) in general. Would you propose to replace that with a mask as well, or just the na_value ?

jreback · 2020-03-27T16:44:55Z

idealy we can just use masks rather than na_value in internal factorize / unique. though that might be some work (maybe?). e.g. easy enough to construct a mask.

It would simplify the hashtable, but I am not sure it would always be beneficial performance-wise. I would need to do some testing to compare pre-computing the mask vs the current implementation to check na_value for dtypes that don't already have the mask available.

And we still have the val != val (in addition to specifying an na_value) in general. Would you propose to replace that with a mask as well, or just the na_value ?

yes its actually easy to do this, just pass mask=isna(obj) in before calling the unique/factorize (the internal ones); will simplify code and likely as performant

jbrockmendel · 2020-03-27T18:37:05Z

pandas/_libs/hashtable_class_helper.pxi.in

@@ -430,7 +439,11 @@ cdef class {{name}}HashTable(HashTable):
            for i in range(n):
                val = values[i]

-                if ignore_na and (
+                if ignore_na and use_mask:


if we're worried about perf for existing cases, could take this check outside of the loop?

We need to check the mask for each value inside the loop, so not sure what can be moved outside?

I was referring to the use_mask check; it would basically become a separate loop or even method

I don't think duplicating the full loop is worth it (the loop itself is 40 lines below here), given the minor performance impact I showed in the timings.

jbrockmendel · 2020-03-27T18:45:57Z

How common do we think it will be for 3rd part EAs to want to use a mask-based implementation? e.g. i expect fletcher will?

If it's just going to be two internal EAs + fletcher, better to just override factorize and not futz with the _values_for_factorize interface.

(more generally, based on other threads, I now think we should get rid of _values_for_factorize and _from_factorized anyway)

yes its actually easy to do this, just pass mask=isna(obj) in before calling the unique/factorize (the internal ones); will simplify code and likely as performant

That becomes a 2-pass algorithm, plus allocates a new array. The latter bothers me more than the former.

jorisvandenbossche · 2020-03-27T19:17:35Z

e.g. i expect fletcher will?

fletcher doesn't implement _values_for_factorize, but has its own factorize() implementation.

How many external projects are otherwise using _values_for_factorize and/or would like to use a mask in that, is hard to estimate (at least GeoPandas is using _values_for_factorize, but wouldn't directly benefit from a mask).

That becomes a 2-pass algorithm, plus allocates a new array. The latter bothers me more than the former.

Yes, I think that is a good reason to not do this universally.

jreback · 2020-03-29T15:34:51Z

needs a rebase

jorisvandenbossche · 2020-04-02T14:41:52Z

For now just override factorize (can have the discussion about having _values_for_factorize optionally return a mask instead of na_value separately)
Moved to the base MaskedArray so also BooleanArray uses it
Added Int64 and boolean to the paramatrization of the factorize benchmark
Added whatsnew

jorisvandenbossche · 2020-04-03T19:46:05Z

pandas/core/arrays/masked.py

+        codes, uniques = _factorize_array(arr, na_sentinel=na_sentinel, mask=mask)
+
+        # the hashtables don't handle all different types of bits
+        uniques = uniques.astype(self.dtype.numpy_dtype, copy=False)


I get a "pandas/core/arrays/masked.py:229: error: "ExtensionDtype" has no attribute "numpy_dtype"" mypy failure

cc @simonjayhawkins @WillAyd how can I solve / silence this? The numpy_dtype attribute is commong for Int/BoolDtype (so I can safely use it), but not for general ExtensionDtype.

Hmm yea so I guess complaining because as far as this class is defined, the return type of self.dtype is an ExtensionDtype (as defined in ExtensionArray)

I guess it comes back to the class design; if we have something else that inherits from BaseMaskedArray it could fail at runtime without if it isn't constructed to return a dtype from self.dtype that has a numpy_dtype attribute, which is a little hefty on the implicitness I guess

So the subclasses IntegerArray and BooleanArray have a correctly typed dtype property. But this method above is defined in their parent class ..

In principle I could add a dtype property

@property def dtype(self) -> Union["IntegerDtype", "BooleanDtype"]: pass

in the BaseMaskedArray class to solve this, I suppose?
But that is also kind of ugly, as the parent class shouldn't really know about its subclasses ..

Right; I think it's going to be tough to make this work with mypy if we implicitly enforce that subclasses make dtype.numpy_dtype available

What does the comment directly preceding it refer to? Perhaps there is a way to do this without breaking the currently implied subclass requirements?

Can you override the type signature of IntegerArray.dtype to be IntegerDtype and BolleanArray.dtype to be BooleanDtype?

Is there a way to just disable mypy on this line?

What does the comment directly preceding it refer to? Perhaps there is a way to do this without breaking the currently implied subclass requirements?

The hashtable is only implemented for int64. So if you have an int32 array, the unique values coming out of _factorize_array are int64, and need to be casted back to int32 (as the uniques returned from this method should be using the original dtype). So for this casting, I need to have access to the dtype's equivalent numpy dtype, which is avalaible as the numpy_dtype attribute.

I could do this differently by eg building up a mapping of EADtypes -> numpy dtypes and looking it up from there instead of using the attribute, but that would just be introducing more complex workarounds to just to satisfy mypy.

How about a MaskedArrayDtype that subclasses ExtensionDtype but has a numpy_dtype property?

Yes, that's probably the cleanest solution architecturally

How about a MaskedArrayDtype that subclasses ExtensionDtype but has a numpy_dtype property?

Yes, that's probably the cleanest solution architecturally.
But the dtype attribute on BaseMaskedArray would still only be a dummy property just to provide typing, since it is overwritten in both subclasses.

But the dtype attribute on BaseMaskedArray would still only be a dummy property just to provide typing

I don't think you'll need to change anything on the array side. The dtypes will inherit from MaskedExtensionDtype, so mypy should know that integer_array.dtype.numpy_type is valid.

Or we add numpy_dtype to the ExtensionDtype API :)

I don't think you'll need to change anything on the array side. The dtypes will inherit from MaskedExtensionDtype, so mypy should know that integer_array.dtype.numpy_type is valid.

No, since mypy thinks self.dtype is an ExtensionDtype, so having IntegerDtype/BooleanDtype inherit from a MaskedDtype that defines this attribute will no help.

So either we would indeed need to add numpy_dtype to the ExtensionDtype API, or I need to add a dummy dtype property on BaseMaskedArray to be able to type it as MaskedDtype.

jreback · 2020-04-05T20:22:55Z

asv_bench/benchmarks/algorithms.py


    def time_factorize(self, unique, sort, dtype):
-        self.idx.factorize(sort=sort)
+        if sort:


isn't this redudant? since sort is a parameter?

self.idx.factorize(sort=sort)

ExtensionArrays don't support the sort keyword, the other values are Index objects, which have that keyword.
So the tests for sort=True are skipped above in case of idx being an EA

this is very confusing then. I would separate the EAs out to a separate asv.

this is very confusing then. I would separate the EAs out to a separate asv.

Agree this is confusing. But I switched to use the factorize function in the hope to make this clearer, and to keep a single benchmark (the index method is just simply calling pd.factorize on itself, so this should benchmark the exact same thing).
And that way, we can actually remove the skip for sort for EAs.

@jreback updated. Much better now I think (and fine for a single benchmark class/function)

jreback · 2020-04-05T20:26:44Z

pandas/core/arrays/masked.py

+        arr = self._data
+        mask = self._mask
+
+        codes, uniques = _factorize_array(arr, na_sentinel=na_sentinel, mask=mask)


is there a reason you want to call a private routine like this directly? shouldn't factorize just handle this directly? (isn't that the point of _values_for_factorize).

This is the way we also do it in the base EA factorize method.
The reason we are using this, and not pd.factorize directly, is because the public factorize does not support the additional na_value and mask keywords.

The point of _values_for_factorize is indeed to avoid that EA authors have to call this private _factorize_array method themselves (and to make it easier to implement EA.factorize), but here, I explicitly do not use the general _values_for_factorize path to be able to customize/optimize the IntegerArray/BooleaArray.factorize() method specifically for those dtypes.

this is really polluting the interface, I would much rather just add they keywords. It seems we are special casing EA to no end. This needs to stop.

The reason we are using this, and not pd.factorize directly, is because the public factorize does not support the additional na_value and mask keywords.

Expanding the public interface of factorize is out of scope for this PR, IMO. The implementation I put here above is exactly how we already do it for 2 years (we are already using _factorize_array in our other EAs) . If you want to do a proposal to change this, please open an issue to discuss.

jreback · 2020-04-06T22:20:13Z

asv_bench/benchmarks/algorithms.py


    def time_factorize(self, unique, sort, dtype):
-        self.idx.factorize(sort=sort)
+        if sort:


this is very confusing then. I would separate the EAs out to a separate asv.

jreback · 2020-04-06T22:21:35Z

pandas/core/arrays/masked.py

+        arr = self._data
+        mask = self._mask
+
+        codes, uniques = _factorize_array(arr, na_sentinel=na_sentinel, mask=mask)


this is really polluting the interface, I would much rather just add they keywords. It seems we are special casing EA to no end. This needs to stop.

The reason we are using this, and not pd.factorize directly, is because the public factorize does not support the additional na_value and mask keywords.

WillAyd · 2020-04-07T15:14:21Z

pandas/core/arrays/masked.py

+        codes, uniques = _factorize_array(arr, na_sentinel=na_sentinel, mask=mask)
+
+        # the hashtables don't handle all different types of bits
+        uniques = uniques.astype(self.dtype.numpy_dtype, copy=False)  # type: ignore


I would prefer not to type: ignore this; I find the current hierarchy of things and the implicit requirements a little wonky

I am happy to do a PR that tries to rework the class hierarchy, but can that be done separately? Because it is really unrelated to the rest of this PR.

I think it is related - this PR introduces an implicit requirement in factorize that self.dtype has a numpy_dtype attribute which. I think mypy is being smart to flag that; would prefer again to not ignore the advice

OK, I added a minimal BaseMaskedDtype class to ensure typing is correct. Can you take a look?

There is no other way to indicate the types to mypy than adding yet another abstract property as I did? (the base ExtensionArray class also has an abstract dtype property, typed as ExtensionDtype, so I am override an abstract property with a new abstract property, just to have the different return type annotation)

I think mypy is right about this one. It only knows (or knew prior to your commit) that BaseMasedArray.dtype is ExtensionDtype, which doesn't necessarily have a .numpy_dtype attribute.

Also, wondering, what is the impact of adding such Generic to the actual class inheritance?

The Generic base class uses a metaclass that defines getitem. types are erased at runtime.

Using generic classes (parameterized or not) to access attributes will result in type check failure. Outside the class definition body, a class attribute cannot be assigned, and can only be looked up by accessing it through a class instance that does not have an instance attribute with the same name

I believe the above restrictions applies to 3.6 as Generic was changed in 3.7 to not use a custom metaclass. see https://docs.python.org/3/library/typing.html#user-defined-generic-types

In the python docs, as far as I can see, Generic is only used to create user defined type variables that then can be used in annotations, but not for subclassing actual classes.

see https://www.python.org/dev/peps/pep-0484/#instantiating-generic-classes-and-type-erasure

@simonjayhawkins since I don't fully grasp the generic, yet, I updated it with a simpler abstract dtype property (see the last added commit), which also seems to fix it. Or you OK with this, for now?

@simonjayhawkins since I don't fully grasp the generic, yet, I updated it with a simpler abstract dtype property (see the last added commit), which also seems to fix it. Or you OK with this, for now?

I'll look in a short while.

I know there is significant resistance to Generic, but it would allow say ExtensionArray[PeriodDtype] to be equivalent to PeriodArray and then AnyArrayLike could then be parameterised to say AnyArrayLike[PeriodDtype]. I suspect at some point we will want to refine types. but that's a discussion for another day.

I am certainly open to going that way if gives better typing in general for EAs and solves a bunch of things. But maybe that can then go into a separate PR (at least I am not doing a # type: ignore anymore ;-))

Or you OK with this, for now?

LGTM

But maybe that can then go into a separate PR

no problem. Generic (and Protocol) are the two typing additions that actually could potentially alter runtime behaviour, so it is right that we scrutinize this. The only problem i've encountered is the repr of type(<instance>) in an assert statement in py3.6, where the instance is an instance of a Generic Class. xref #31574

at least I am not doing a # type: ignore anymore ;-)

👍

jorisvandenbossche · 2020-05-01T12:36:30Z

Were there other remaining comments (apart from the mypy issues) ?
As far as I look back at the discussion, from my side everything seems accounted for.

jorisvandenbossche · 2020-05-09T10:01:00Z

Given no further comments, going to merge this.

ENH/PERF: use mask in factorize

246b787

jorisvandenbossche added Algos Non-arithmetic algos: value_counts, factorize, sorting, isin, clip, shift, diff NA - MaskedArrays Related to pd.NA and nullable extension arrays Performance Memory or execution speed performance labels Mar 27, 2020

jorisvandenbossche added this to the 1.1 milestone Mar 27, 2020

jorisvandenbossche commented Mar 27, 2020

View reviewed changes

fix typing

af1cdea

jorisvandenbossche requested review from jreback and jbrockmendel March 27, 2020 16:20

jreback requested changes Mar 27, 2020

View reviewed changes

jbrockmendel reviewed Mar 27, 2020

View reviewed changes

jorisvandenbossche added 3 commits April 2, 2020 16:05

Merge remote-tracking branch 'upstream/master' into masked-hashtable-int

d030ff1

override in factorize, moved to base masked

be5a21d

add Int64/boolean to factorize benchmarks

b0a88b9

add whatsnew

2e94842

jorisvandenbossche commented Apr 3, 2020

View reviewed changes

jorisvandenbossche mentioned this pull request Apr 3, 2020

EA interface - requirements for "hashable, value+order-preserving ndarray" #33276

Open

jorisvandenbossche added 2 commits April 4, 2020 09:14

Merge remote-tracking branch 'upstream/master' into masked-hashtable-int

bcde548

add # type: ignore

c97d357

jreback requested changes Apr 5, 2020

View reviewed changes

jreback requested changes Apr 6, 2020

View reviewed changes

jorisvandenbossche added 2 commits April 7, 2020 08:51

Merge remote-tracking branch 'upstream/master' into masked-hashtable-int

175a8f6

update benchmark

a6bc6fc

WillAyd requested changes Apr 7, 2020

View reviewed changes

jorisvandenbossche added 4 commits April 7, 2020 22:33

add BaseMaskedDtype

889b1b9

use class variable annotation

0370045

Merge remote-tracking branch 'upstream/master' into masked-hashtable-int

24b528f

use abstract dtype

6ed5239

Merge remote-tracking branch 'upstream/master' into masked-hashtable-int

1f7e994

jorisvandenbossche merged commit 9ed015f into pandas-dev:master May 9, 2020

jorisvandenbossche deleted the masked-hashtable-int branch May 9, 2020 11:41

rhshadrach pushed a commit to rhshadrach/pandas that referenced this pull request May 10, 2020

ENH/PERF: use mask in factorize for nullable dtypes (pandas-dev#33064)

89257e2

jorisvandenbossche mentioned this pull request Jul 15, 2020

Overview of impact / achievements of the CZI grant (round 1) TomAugspurger/pandas-czi#6

Open

This was referenced Feb 2, 2022

ENH: support masked arrays in hashtable-based functions (duplicated, isin, mode) #45776

Closed

ENH: add mask-aware implementation of factorize algos #30037

Closed

ENH/PERF: use mask in factorize for nullable dtypes #33064

ENH/PERF: use mask in factorize for nullable dtypes #33064

Conversation

jorisvandenbossche commented Mar 27, 2020 • edited Loading

jorisvandenbossche commented Mar 27, 2020

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

jreback left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jorisvandenbossche commented Mar 27, 2020

jreback commented Mar 27, 2020

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

jbrockmendel commented Mar 27, 2020

jorisvandenbossche commented Mar 27, 2020

jreback commented Mar 29, 2020

jorisvandenbossche commented Apr 2, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jorisvandenbossche Apr 3, 2020 • edited Loading

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 May 1, 2020 • edited Loading

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 May 1, 2020

jorisvandenbossche commented May 9, 2020

jorisvandenbossche commented Mar 27, 2020 •

edited

Loading

jorisvandenbossche commented Apr 2, 2020 •

edited

Loading

jorisvandenbossche Apr 3, 2020 •

edited

Loading

jorisvandenbossche May 1, 2020 •

edited

Loading