Plotly.react #2341

alexcjohnson · 2018-02-06T22:57:43Z

Introduces Plotly.react - an idempotent, full-state, self-diffing update method with a signature matching Plotly.newPlot but using the faster update pathways of Plotly.restyle/relayout/update.

Diffing is done on fullData and fullLayout, not on data and layout, for a few reasons:

We don't copy data and layout, so there's nothing to diff there if the user modifies them in place. But _full* get regenerated as new objects with every supplyDefaults call (which must happen on every update regardless).
No need to worry about the data types we're diffing, as supplyDefaults cleans that up. Also we won't diff user attributes that don't go into the plot, or irrelevant leftovers of old plot types.

We do NOT dive into data arrays (valType: 'data_array' or arrayOk: true) in the diffing algorithm. There are two ways to tell Plotly.react that the data have changed:

layout.datarevision: can take any value - string, number, whatever - and if it is not === its previous value, we treat the data as changed. Data arrays themselves are ignored during diffing. Note that right now there's no difference in the pathway we'd take for one vs all data arrays changing, but in the future if we implement partial recalc we can make a similar flag for each array, or perhaps each trace.
If you omit datarevision, we assume data arrays are being used as immutable and compare them with === to determine if anything changed. This has some known issues (resulting in slow recalc when it wouldn't be needed) that we should fix:
- At least one trace type (surface) fills in index arrays for missing dimensions. These are never === each other. We should provide these later in the pipeline, not attached to _fullData, or fill them in as _private attributes which do not get diffed.
- At least one trace type (scatter) will slice uneven arrays to the same length, which will also make them compare unequal. We should just record a private _length or something instead.

see #1850
cc @etpinard @chriddyp @nicolaskruchten @VeraZab @jackparmer

alexcjohnson · 2018-02-06T23:03:38Z

src/components/legend/defaults.js

@@ -58,6 +56,8 @@ module.exports = function legendDefaults(layoutIn, layoutOut, fullData) {

 if(showLegend === false) return;

+ layoutOut.legend = containerOut;


legend's visibility is controlled by showlegend, which is outside the legend container itself, unlike most of our newer containers that have visible inside the container. Unless and until we change that (v2?) the legend container should not exist when it's not needed. This was causing me problems as relinkPrivateKeys was humorously removing newFullLayout.legend if it was empty and oldFullLayout.legend existed... so its existence would toggle on Plotly.react calls, making it look like the legend was always in need of updating 💫

Very nice catch.

until we change that (v2?)

I'd vote +1 here.

added to #420

the legend container should not exist when it's not needed.

so by default, in _fullLayout, the .legend key wouldn't be there?
we read from _fullLayout to display widgets in the editor, if there's no .legend key, or a ._fullLayout.legend.visible key, then the widget won't display..

We could have a work around, but it's kind of one of the basic premises of widget visibility in the editor..

@VeraZab you must not have been dependent on the existence of a legend container up to now, as it was unreliable ^^ - showlegend is still there, but now it's reliably the case that if _fullLayout.showlegend=false there will be NO _fullLayout.legend container.

alexcjohnson · 2018-02-06T23:07:50Z

src/plot_api/plot_api.js

@@ -1632,26 +1639,6 @@ function _restyle(gd, aobj, traces) {
 }
 }

- // check axes we've flagged for possible deletion
- // flagAxForDelete is a hash so we can make sure we only get each axis once
- var axListForDelete = Object.keys(flagAxForDelete);


This isn't really related to Plotly.react but noticed as I was working here: post #2227 we don't need to delete unused axes anymore - in fact we don't want to.

Oh but it's not self-contained here - I need to get rid of flagAxForDelete above...

finished deleting in aab4f11

This has most probably caused:
#2392

alexcjohnson · 2018-02-06T23:11:04Z

src/plot_api/plot_api.js

+
+ // you can use this as the initial draw as well as to update
+ if(!Lib.isPlotDiv(gd) || !oldFullData || !oldFullLayout) {
+ plotDone = Plotly.newPlot(gd, data, layout, config);


Plotly.react is a one-stop-shop, it can replace Plotly.newPlot and Plotly.plot entirely.

the dream

alexcjohnson · 2018-02-06T23:17:03Z

src/plot_api/plot_api.js

+ if(restyleFlags.calc || relayoutFlags.calc) gd.calcdata = undefined;
+
+ // Note: what restyle/relayout use impliedEdits and clearAxisTypes for
+ // must be handled by the user when using Plotly.react.


⚠️ not sure if there's something we can do to make this easier for users (pull out the relevant sections from _restyle and _relayout perhaps into helper functions?), but I don't think we should be trying to do anything with impliedEdits or clearAxisTypes inside Plotly.react.

Can you clarify what you mean by: must be handling by user?

impliedEdits are things like turning off axis.autorange when you set an explicit range. If you use relayout et al we do that for you, so Plotly.relayout(gd, 'yaxis.range', [5, 10]) automatically sets yaxis.autorange: false - otherwise the range wouldn't actually change, as autorange would put it back to what you started with.

The equivalent with Plotly.react would be:

layout.yaxis.range = [5, 10]; // you have to explicitly disable autorange when using `Plotly.react` // even if you never explicitly enabled it in the first place, since plotly.js // inserted `yaxis.autorange: true` in your layout object layout.yaxis.autorange = false; Plotly.react(gd, data, layout);

clearAxisTypes is similar: using Plotly.restyle(gd, 'x', [['a', 'b', 'c']], [0]) will clear the axis types of the axes trace 0 is displayed on, so the autotype algorithm runs again. With Plotly.react you'll have to explicitly remove xaxis.type if you want to invoke autotype.

I suppose there's a discussion to be had about this: it would certainly be nice to just make the change you want, the same as you do with relayout, and have us figure out what we need to do to make it happen. On the other hand, with Plotly.react you're saying "this is the new plot state I want rendered", and for us to comb through it and say "Oh, I see you made this change, you must have wanted this other change too" seems heavy-handed and error-prone - what if you really don't want that other change, there's no way to tell us that (as there would be with relayout eg Plotly.relayout(gd, {'yaxis.range': [5, 10], 'yaxis.autorange': true}) (which seems a contrived example, but you might actually do that for example to turn the range from decreasing autorange to increasing autorange).

So rather than bake this into Plotly.react as we do with relayout et al, I think we should try and find a way to help the user access the recommendations we would make with impliedEdits and clearAxisTypes, but leave it up to them whether to follow those recommendations.

what if you really don't want that other change, there's no way to tell us that

Thanks for writing this. This makes 💯 % total sense 📚

think we should try and find a way to help the user access the recommendations we would make with impliedEdits and clearAxisTypes, but leave it up to them whether to follow those recommendations.

Perhaps all we need is to (under a certain config option) log whenever we mutate user data or user layout. Alternatively, Plotly.validate could be use here e.g.

Plotly.validate([{ y: [1, 2, 1] }], { xaxis: { autorange: true, range: [1, 2] } })

could log something like: increasing range detected, range values ignored under autorange. Then together with #1741, it should make it fairly easy for plotly.js users to debug.

alexcjohnson · 2018-02-06T23:17:55Z

src/plot_api/plot_api.js

+ if(frames) {
+ gd._transitionData = {};
+ Plots.createTransitionData(gd);
+ seq.push(addFrames);


anyone want to comment on how we should handle frames here?

Looks good to me 👌

alexcjohnson · 2018-02-06T23:23:05Z

src/plot_api/plot_api.js

+ gd.data = data || [];
+ helpers.cleanData(gd.data, []);
+ gd.layout = layout || {};
+ helpers.cleanLayout(gd.layout);


I'm a little ambivalent about this - there's part of cleanData, fillling in uids, that seems like it may not even belong to cleanData but anyway we need to do it. But the rest, do we need to be cleaning obsolete usages on every Plotly.react call? These routines mutate the inputs so after the first call the user shouldn't have any of them left; that said users can always provide totally new data/layout...

I agree, the uid generation routine should be taken out. It has no place in cleanData.

In v2, I'm thinking cleanData and cleanLayout should be called only under a special config flag (e.g. compatibility: true, or a more granular compatibilityLevel: 'v1') whereas assigning uid to traces will always have to be done.

cc #2007

alexcjohnson · 2018-02-06T23:31:29Z

src/plots/cartesian/graph_interact.js

- if((!fullLayout._has('cartesian') && !fullLayout._has('gl2d')) || gd._context.staticPlot) return;
+ if(gd._context.staticPlot) {
+ // this sweeps up more than just cartesian drag elements...
+ d3.select(gd).selectAll('.drag').remove();


Far as I could tell this is the only change needed so config can be altered with a regular Plotly.plot call - everything else I tried just seemed to work. If we make more of a supplyDefaults type framework for config at some point we may be able to minimize the update pathway there too, like modebar-only changes... but it's probably not a very common use case anyhow.

What about other svg-based subplot types? For example, I'm thinking we could add a similar .remove() statement for polar here

we could... though this one kills the polar draggers too, hence my comment. I originally put the remove inside the one if((!fullLayout._has('cartesian') && !fullLayout._has('gl2d')) || gd._context.staticPlot) block, and it broke all polar drag and hover effects.

You were saying something about overly broad selectAll statements the other day 😏 It wasn't obvious to me how to make this more specific though...

though this one kills the polar draggers too,

Oh nice. Is this tested? If not, would you mind 🔒 ing this down?

🔒 0548db3

alexcjohnson · 2018-02-06T23:33:34Z

src/plots/plots.js

- newData = gd.data || [];
+ if(oldFullLayout._skipSD) {
+ delete oldFullLayout._skipSD;
+ return;


my hack to prevent running supplyDefaults twice in full-replot update pathways.

Nice! Would you mind renaming it _skipDefaults? SD didn't ring a 🔔 for me at first sight, so I suspect it may confuse other devs.

good call -> _skipDefaults in 1074804

so we can use it to make index arrays

alexcjohnson · 2018-02-07T03:46:42Z

src/lib/push_unique.js

@@ -30,7 +32,7 @@ module.exports = function pushUnique(array, item) {
 }
 array.push(item);
 }
- else if(item && array.indexOf(item) === -1) array.push(item);
+ else if((item || item === 0) && array.indexOf(item) === -1) array.push(item);


Any problems with this? I'm assuming falsy items were mostly ignored for data filtering purposes, which also should accept 0 I would think...

None from me. Let's just make sure it's documented in this its test suite.

Oh oops, that's done already. Thanks!

alexcjohnson · 2018-02-07T03:50:02Z

@etpinard I still don't understand why I have this big package-lock diff - I can remove it from the PR, but I'd much rather get to a situation where I'm not constantly generating a different one from what's in the repo...

etpinard

Well, this is pretty great. 🎉

I made a few comments and I'll take this PR out for a test drive tomorrow morning, but yeah, I'm really liking the strategy used.

About the package-lock file, I have no idea what's up. What node and npm versions are you using? Did you rebase off the freshest master before rm -rf node_modules && npm i?

etpinard · 2018-02-07T03:55:24Z

src/plots/plots.js

- newData = gd.data || [];
+ if(oldFullLayout._skipSD) {
+ delete oldFullLayout._skipSD;
+ return;


Nice! Would you mind renaming it _skipDefaults? SD didn't ring a 🔔 for me at first sight, so I suspect it may confuse other devs.

etpinard · 2018-02-07T03:56:33Z

test/jasmine/tests/plot_api_test.js

+
+ function countCalls(counts) {
+ var callsFinal = Lib.extendFlat({}, counts);
+ // TODO: do we really need to do layoutStyles twice in Plotly.plot?


I hope not 😛

I suspect this is about automargins - right now the drawing pipeline is like:

draw everything that can alter margins

check if anything is too big and needs to change margins

draw that stuff again if the margins changed

then draw all the stuff that can't change the margins

we really should refactor it to something like:

calculate the sizes of everything that can alter margins, without actually drawing it

calculate all the automargins (which determines the positions of everything)

draw everything

or perhaps:

draw everything that can alter margins, but don't position it

calculate all the automargins

position the predrawn items

draw everything else

etpinard · 2018-02-07T04:00:15Z

test/jasmine/tests/plot_api_test.js

+ countCalls({plot: 1});
+
+ // ideally we'd just do this with `surface` but colorbar attrs have editType 'calc' there
+ // TODO: can we drop them to type: 'colorbars' even for the 3D types?


Yeah, this should work with editType: 'colorbars'.

etpinard · 2018-02-07T04:02:16Z

src/components/annotations/attributes.js

@@ -20,7 +20,7 @@ module.exports = {
 valType: 'boolean',
 role: 'info',
 dflt: true,
- editType: 'calcIfAutorange',
+ editType: 'calcIfAutorange+arraydraw',


Can you clarify why we now need arraydraw here and in shapes/attributes.js?

relayout uses containerArrayMatch to figure out what edits need to be handled with the special add/remove/edit syntax, and arraydraw was just a placeholder for "relayout will handle this, don't worry about it".

Here we're letting the user do their own editing, so there's much less of that complexity. So now I use editType: 'arraydraw' as the signal to try and draw components using their draw or drawOne component methods, hence it has to actually be there!

Very clear. Thanks 📚

etpinard · 2018-02-07T04:04:02Z

src/components/legend/defaults.js

@@ -58,6 +56,8 @@ module.exports = function legendDefaults(layoutIn, layoutOut, fullData) {

 if(showLegend === false) return;

+ layoutOut.legend = containerOut;


Very nice catch.

until we change that (v2?)

I'd vote +1 here.

etpinard · 2018-02-07T04:17:14Z

src/plot_api/plot_api.js

+ if(restyleFlags.calc || relayoutFlags.calc) gd.calcdata = undefined;
+
+ // Note: what restyle/relayout use impliedEdits and clearAxisTypes for
+ // must be handled by the user when using Plotly.react.


Can you clarify what you mean by: must be handling by user?

etpinard · 2018-02-07T04:37:08Z

src/plot_api/plot_api.js

+ if(frames) {
+ gd._transitionData = {};
+ Plots.createTransitionData(gd);
+ seq.push(addFrames);


Looks good to me 👌

etpinard · 2018-02-07T04:38:23Z

src/plots/layout_attributes.js

+ 'incremental change.',
+ 'If NOT provided, `Plotly.react` assumes that data arrays are',
+ 'being treated as immutable, thus any data array with a',
+ 'different identity from its predecessor contains new data.'


This is brilliant ❇️

yes, yes, yes!

etpinard · 2018-02-07T04:40:01Z

src/plots/cartesian/graph_interact.js

- if((!fullLayout._has('cartesian') && !fullLayout._has('gl2d')) || gd._context.staticPlot) return;
+ if(gd._context.staticPlot) {
+ // this sweeps up more than just cartesian drag elements...
+ d3.select(gd).selectAll('.drag').remove();


What about other svg-based subplot types? For example, I'm thinking we could add a similar .remove() statement for polar here

etpinard · 2018-02-07T04:41:30Z

src/lib/push_unique.js

@@ -30,7 +32,7 @@ module.exports = function pushUnique(array, item) {
 }
 array.push(item);
 }
- else if(item && array.indexOf(item) === -1) array.push(item);
+ else if((item || item === 0) && array.indexOf(item) === -1) array.push(item);


Oh oops, that's done already. Thanks!

nicolaskruchten · 2018-02-07T18:08:47Z

I don't know enough about the implementation at this point to comment on that, but the approach and API look pretty awesome from the point of view of react-plotly.js :)

width and height looked to Plotly.react like attributes -> _width, _height

so as to not muck up plotly.react immutable logic

instead just record the length as fullTrace._length

…ults

chriddyp · 2018-02-08T21:41:27Z

This is really great, the design with datarevision is really great and Dash users will definitely be able to use.

A couple questions:

If the datarevision remains the same, can the user omit the data?
i.e.

newPlot(gd, [{'x': [1, 2, 3], 'y': [3, 1, 2]}], {'datarevision': 1})
react(gd, [{'marker': {'color': 'blue'}}], {'datarevision': 1})

Would that work? If so, then Dash users could potentially omit their data from even being transported if it remained the same.

Does plotly.react keep the zoom layer or reset it? i.e. if the user zooms in on a map, and then changes a dropdown, will the graph reset or not? I believe that it should (since that is what newPlot currently does).
Do you imagine that datarevision will eventually allow specificity of which trace's data changed?

the range_slider_multiple mock showed the first bug, but I updated it so neither bound matches the default [-1, 6] second bug was that impliedEdits were missing so some range updates wouldn't work

NaN !== NaN wat was breaking Plotly.react.

🤦 transforms...

thank you airfoil.json for including a config arg

alexcjohnson · 2018-02-08T23:10:05Z

Re: image diffs - I can't tell why gl3d_ribbons changed... looks like a subpixel change but I don't know what caused it. But I think gl2d_axes_labels2 was incorrect before and is more correct now (diff / new) (it has fill to zero so the autorange should go to zero. The autorange actually goes a little past zero now, rather than ending exactly at zero as it does with regular scatter, but the x axis is also a little different in the two cases. Again I'm not sure what I did that changed it though...

no idea why this changed...

and then npm i using node 8.9.4, npm 5.6.0, so it seems to last...

There are a bunch of extra edge cases here if the x/y/z arrays don't have one of the expected sizes/dimensionalities... but this covers more than it used to anyhow, and gets the ribbons mock right.

alexcjohnson · 2018-02-09T04:00:32Z

src/traces/surface/defaults.js

 coerce('y');

+ traceOut._xlength = (Array.isArray(x) && Array.isArray(x[0])) ? z.length : z[0].length;
+ traceOut._ylength = z.length;


_xlength and _ylength are used in two places, to try and describe how much of each array to use - one in surface/convert (only for the surface type) and again in gl3d/scene (for all 3d types). The two do things in different ways, and especially since surface can use x/y arrays of either 1 or 2 dimensions, they weren't getting things right. I was trying to do better at handling mismatched arrays but this at least gets it right again for arrays of the correct length (and brings back the original gl3d_ribbons mock), and I've added an item for surface to #2353 to explore the mismatched cases more fully.

etpinard · 2018-02-09T15:04:14Z

Awesome stuff 💃

Follow and/or comment on #2353 for more on this topic.

alexcjohnson · 2018-02-09T15:32:51Z

@chriddyp

If the datarevision remains the same, can the user omit the data?
i.e.
newPlot(gd, [{'x': [1, 2, 3], 'y': [3, 1, 2]}], {'datarevision': 1})
react(gd, [{'marker': {'color': 'blue'}}], {'datarevision': 1})
Would that work? If so, then Dash users could potentially omit their data from even being transported if it remained the same.

Sounds dangerous to me. In some cases this would work, if you insert empty arrays for the data (if you don't even do that, we'll see no array vs used-to-be-an-array and do a recalc regardless of datarevision). But if anything else in the diff triggers a recalc (like, for example, changing the overall plot size, or autoranging an axis), we'll pick up the omitted data anyway and you're hosed.

Does plotly.react keep the zoom layer or reset it? i.e. if the user zooms in on a map, and then changes a dropdown, will the graph reset or not? I believe that it should (since that is what newPlot currently does).

Plotly.react itself does nothing about the reverse data flow - UI-driven changes to the plot pushing back up into the plot state you're holding onto elsewhere - though since these UI-driven changes mutate gd.data and gd.layout and most of the time these are the same objects you provided with Plotly.newPlot (or Plotly.react), you may in fact have these changes in your state whether you wanted them or not (probably not with Dash though, as I'm assuming a new plot state coming from Python would generate new data and layout objects). If you've picked up these changes and included them in the new state you send to Plotly.react, then you'll see them persist on the plot. If you haven't, you won't.

Do you imagine that datarevision will eventually allow specificity of which trace's data changed?

Yes, as mentioned in the opening comment:

Note that right now there's no difference in the pathway we'd take for one vs all data arrays changing, but in the future if we implement partial recalc we can make a similar flag for each array, or perhaps each trace.

etpinard · 2018-02-09T15:45:33Z

cc @VeraZab @nicolaskruchten this will part of 1.34.0 that we'll release on Monday (100% guaranteed this time I promise). Sorry for the wait.

Plotly.react

54b6969

alexcjohnson commented Feb 6, 2018

View reviewed changes

alexcjohnson added 3 commits February 6, 2018 18:42

remove flagAxForDelete from _restyle

aab4f11

allow 0 in pushUnique

45c56b0

so we can use it to make index arrays

test Plotly.react with component arrays

e58b74d

alexcjohnson commented Feb 7, 2018

View reviewed changes

alexcjohnson added feature something new status: reviewable labels Feb 7, 2018

etpinard reviewed Feb 7, 2018

View reviewed changes

alexcjohnson mentioned this pull request Feb 7, 2018

wishlist for potential breaking changes since v1 #420

Closed

15 tasks

_skipSD -> _skipDefaults

1074804

etpinard added this to the v1.34.0 milestone Feb 7, 2018

test of polar toggle staticPlot

0548db3

alexcjohnson added 8 commits February 7, 2018 13:23

multiline if statements for better debugging

698fa4f

only add _private attrs to legend after supplyDefaults

3c56243

width and height looked to Plotly.react like attributes -> _width, _height

cleanData should modify textposition in place

f01bab9

so as to not muck up plotly.react immutable logic

don't slice uneven arrays in scatter and related traces

2fbd97c

instead just record the length as fullTrace._length

stop slicing and generating fake arrays in scatter3d and surface defa…

f873b26

…ults

remove old obsolete exponentformat/showexponent fallback

5aaa0df

cleaner handling of bin defaults

7a3b075

test_dashboard: highlight the displayed mock in search results

5814642

review comments - 🔪 commented-out code, comment on cleanData

ade3cca

alexcjohnson added 12 commits February 8, 2018 17:41

fixes for mapbox with Plotly.react

4b98be6

update polar for Plotly.react compatibility

3178b2e

stop filling new [] in pie defaults

564dff6

fix bugs in rangeslider with explicit range and relayout

be4932d

the range_slider_multiple mock showed the first bug, but I updated it so neither bound matches the default [-1, 6] second bug was that impliedEdits were missing so some range updates wouldn't work

clean up rangeselector for Plotly.react

0752aa5

update scatterternary for Plotly.react

9ba4be9

avoid NaN in (un)selected.marker.opacity

9795b69

NaN !== NaN wat was breaking Plotly.react.

fix for transformed histograms to give consistent _fullInput

57c7d6c

🤦 transforms...

more complete diffing of config

3a11062

thank you airfoil.json for including a config arg

update scattergeo test

896471c

wider acceptance range for toimage test

e12ce1b

more complete set of Plotly.react noop mocks to test

46c6642

alexcjohnson mentioned this pull request Feb 8, 2018

open items uncovered during Plotly.react push #2353

Closed

7 tasks

lint

4197c32

alexcjohnson added 4 commits February 8, 2018 20:25

fix scattergl uneven array handling

f9d6151

updated gl3d_ribbons mock

c8a07e8

no idea why this changed...

reset package-lock to master

2a7d640

and then npm i using node 8.9.4, npm 5.6.0, so it seems to last...

get the right _xlength for surface if x is 2D

5d6a4a0

There are a bunch of extra edge cases here if the x/y/z arrays don't have one of the expected sizes/dimensionalities... but this covers more than it used to anyhow, and gets the ribbons mock right.

alexcjohnson commented Feb 9, 2018

View reviewed changes

alexcjohnson merged commit abbb67d into master Feb 9, 2018

alexcjohnson deleted the react branch February 9, 2018 15:33

alexcjohnson mentioned this pull request Feb 12, 2018

a version of .newPlot that does the .update logic behind the scenes #1850

Closed

igorpejic mentioned this pull request Feb 26, 2018

Axes not redrawing after layout change plotly/react-plotly.js#45

Closed

chriddyp mentioned this pull request Aug 28, 2018

Fix #260, allow '{}' as Graph.figure plotly/dash-core-components#285

Merged

		@@ -58,6 +56,8 @@ module.exports = function legendDefaults(layoutIn, layoutOut, fullData) {

		if(showLegend === false) return;

		layoutOut.legend = containerOut;

Plotly.react #2341

Plotly.react #2341

Conversation

alexcjohnson commented Feb 6, 2018

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexcjohnson commented Feb 7, 2018

etpinard 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

nicolaskruchten commented Feb 7, 2018

chriddyp commented Feb 8, 2018

alexcjohnson commented Feb 8, 2018

Choose a reason for hiding this comment

etpinard commented Feb 9, 2018

alexcjohnson commented Feb 9, 2018

etpinard commented Feb 9, 2018 • edited Loading

etpinard commented Feb 9, 2018 •

edited

Loading