Plotly.react sequence and transforms #2508

Activity

1. alexcjohnson

   added

   bugsomething brokensomething broken

   on Mar 29, 2018
2. nicolaskruchten commented on Apr 4, 2018

   

   nicolaskruchten

   on Apr 4, 2018

   ContributorAuthor

   More actions

   Related to #2470?
3. alexcjohnson commented on Apr 5, 2018

   

   alexcjohnson

   on Apr 5, 2018

   Last edited by alexcjohnson

   Collaborator

   More actions

   Two tests to add, probably to transform_groupby_test, when fixing this:

   • relinkPrivateKeys React finance precursor #2525 (comment)
   • Plotly.react diffing on fullInput React finance precursor #2525 (comment)
4. alexcjohnson commented on Apr 19, 2018

   

   alexcjohnson

   on Apr 19, 2018

   Collaborator

   More actions

   A new rule I'm considering implementing & enforcing, to help ensure transforms work correctly with all trace types:

   • Every trace must define _length, and MUST do it during supplyDefaults so it's available before any transforms operate.
   • This is (at least for now) only appropriate for traces that have 1D data - so it applies for example to heatmaps if you provide triplet data (x/y/z are all 1D) but not if z is 2D.
   • When it does not apply, set _length = null, and all transforms that operate on the assumption of 1D data (which is currently all of them) will be automatically disabled.

   @etpinard thoughts?
5. etpinard commented on Apr 19, 2018

   

   etpinard

   on Apr 19, 2018

   Contributor

   More actions

   I like it. It sounds like a nice way to 🔒 the current behavior. If ever we start supporting transforms on 2D arrays, we can reassess.

   🎉React with 🎉1Reacted by Alex Johnson
6. alexcjohnson commented on Apr 19, 2018

   

   alexcjohnson

   on Apr 19, 2018

   Collaborator

   More actions

   If ever we start supporting transforms on 2D arrays, we can reassess.

   Yes - just looking at table traces... it would be sweet to be able to filter/sort/aggregate a table, but this has (at least) two problems right now:

   • cells.values is a 2D array... our transforms wouldn't know what to do with this, and generally you'd want the transform target to be one column of that, but we don't have a mechanism to pick out a column.
   • header.values is also a 2D array, so it'll show up in _arrayAttrs and if we try to continue using automatic array collection we'll have to find a way to exclude headers.

   Anyway trying to generalize this to other 2D trace types seems tough, given all the different combinations of 1D & 2D data available within a single trace type, with some of the 1D arrays matching columns of the 2D, some matching rows, some allowing 1D or 2D, etc etc... So either each transform would have to be specifically extended for each 2D trace type it wants to support or we'd need a way to describe (in the schema?) which dimensions of which attributes relate to each other.

   So with _length = null we can disable this explicitly, then re-enable it type-by-type as we figure it out.
7. etpinard commented on Apr 19, 2018

   

   etpinard

   on Apr 19, 2018

   Contributor

   More actions

   So with _length = null we can disable this explicitly, then re-enable it type-by-type as we figure it out.

   Great 👌
8. alexcjohnson

   added 2 commits that reference this issue on Apr 22, 2018

   fix #2508, fix #2470 - problems with Plotly.react and aggregate trans…

   ...

   This issue will close once commit fcc459d is merged into the 'main' branch.

   fcc459d

   fixes and test for checklist in #2508

   This issue will close once commit 965bcfb is merged into the 'main' branch.

   965bcfb
9. alexcjohnson

   closed this as completedin fcc459don Apr 26, 2018