Multicategory sorting fixes by etpinard · Pull Request #3362 · plotly/plotly.js

etpinard · 2018-12-21T15:57:08Z

fixes#3356 - and made me realised that multicategory are slightly more complicated than first expected.

In brief, we need to (1) scan the data array (2) sort the resulting multi-category list (3) loop over the data arrays again to retrieve their calcdata values.

cc @plotly/plotly_js

- so that we handle cross-trace sorting correctly

alexcjohnson · 2018-12-21T16:14:21Z


 // clear stuff that should recomputed in 'regular' loop
- if(hasCalcTransform) clearAxesCalc(axList);
+ if(hasCalcTransform) setupAxisCategories(axList);


, fullData?

maybe a can of worms, but is there a test for multicat + calctransforms (or non-calc-transforms for that matter)?

Multicategory axes don't support transforms. I'm not sure how gracefully things fail though.

OK, that's fine. Nothing more from me - I'll let one of the other folks give this a once-over and the 💃

Transforms + multicategory tracked in -> #3363

alexcjohnson · 2018-12-21T16:17:48Z

+ ax.clearCalc();
+ if(ax.type === 'multicategory') {
+ ax.setupMultiCategory(fullData);
+ }


clearCalc is also used in rangeslider.draw and gl2d/scene2d - do these need setupMultiCategory? Should that just be part of clearCalc for multicategory axes?

clearCalc is also used in rangeslider.draw and gl2d/scene2d - do these need setupMultiCategory?
setupMultiCategory only needs to be called once (here in doCalcdata, to fill in ax._muticatList), so rangeslider don't need it, that's why I didn't put the setupMultiCategory logic in clearCalc. As for gl2d/scene2d, gl2d subplots don't support multicategory axes, so I think we're 👌

archmoj · 2018-12-21T20:19:46Z

💃

etpinard · 2018-12-21T15:57:08Z

fixes#3356 - and made me realised that multicategory are slightly more complicated than first expected.

In brief, we need to (1) scan the data array (2) sort the resulting multi-category list (3) loop over the data arrays again to retrieve their calcdata values.

cc @plotly/plotly_js

- so that we handle cross-trace sorting correctly

alexcjohnson · 2018-12-21T16:14:21Z


 // clear stuff that should recomputed in 'regular' loop
- if(hasCalcTransform) clearAxesCalc(axList);
+ if(hasCalcTransform) setupAxisCategories(axList);


, fullData?

maybe a can of worms, but is there a test for multicat + calctransforms (or non-calc-transforms for that matter)?

Multicategory axes don't support transforms. I'm not sure how gracefully things fail though.

OK, that's fine. Nothing more from me - I'll let one of the other folks give this a once-over and the 💃

Transforms + multicategory tracked in -> #3363

alexcjohnson · 2018-12-21T16:17:48Z

+ ax.clearCalc();
+ if(ax.type === 'multicategory') {
+ ax.setupMultiCategory(fullData);
+ }


clearCalc is also used in rangeslider.draw and gl2d/scene2d - do these need setupMultiCategory? Should that just be part of clearCalc for multicategory axes?

clearCalc is also used in rangeslider.draw and gl2d/scene2d - do these need setupMultiCategory?
setupMultiCategory only needs to be called once (here in doCalcdata, to fill in ax._muticatList), so rangeslider don't need it, that's why I didn't put the setupMultiCategory logic in clearCalc. As for gl2d/scene2d, gl2d subplots don't support multicategory axes, so I think we're 👌

archmoj · 2018-12-21T20:19:46Z

💃

etpinard · 2018-12-21T15:57:08Z

fixes#3356 - and made me realised that multicategory are slightly more complicated than first expected.

In brief, we need to (1) scan the data array (2) sort the resulting multi-category list (3) loop over the data arrays again to retrieve their calcdata values.

cc @plotly/plotly_js

- so that we handle cross-trace sorting correctly

alexcjohnson · 2018-12-21T16:14:21Z


 // clear stuff that should recomputed in 'regular' loop
- if(hasCalcTransform) clearAxesCalc(axList);
+ if(hasCalcTransform) setupAxisCategories(axList);


, fullData?

maybe a can of worms, but is there a test for multicat + calctransforms (or non-calc-transforms for that matter)?

Multicategory axes don't support transforms. I'm not sure how gracefully things fail though.

OK, that's fine. Nothing more from me - I'll let one of the other folks give this a once-over and the 💃

Transforms + multicategory tracked in -> #3363

alexcjohnson · 2018-12-21T16:17:48Z

+ ax.clearCalc();
+ if(ax.type === 'multicategory') {
+ ax.setupMultiCategory(fullData);
+ }


clearCalc is also used in rangeslider.draw and gl2d/scene2d - do these need setupMultiCategory? Should that just be part of clearCalc for multicategory axes?

clearCalc is also used in rangeslider.draw and gl2d/scene2d - do these need setupMultiCategory?
setupMultiCategory only needs to be called once (here in doCalcdata, to fill in ax._muticatList), so rangeslider don't need it, that's why I didn't put the setupMultiCategory logic in clearCalc. As for gl2d/scene2d, gl2d subplots don't support multicategory axes, so I think we're 👌

archmoj · 2018-12-21T20:19:46Z

💃

etpinard · 2018-12-21T15:57:08Z

fixes#3356 - and made me realised that multicategory are slightly more complicated than first expected.

In brief, we need to (1) scan the data array (2) sort the resulting multi-category list (3) loop over the data arrays again to retrieve their calcdata values.

cc @plotly/plotly_js

- so that we handle cross-trace sorting correctly

alexcjohnson · 2018-12-21T16:14:21Z


 // clear stuff that should recomputed in 'regular' loop
- if(hasCalcTransform) clearAxesCalc(axList);
+ if(hasCalcTransform) setupAxisCategories(axList);


, fullData?

maybe a can of worms, but is there a test for multicat + calctransforms (or non-calc-transforms for that matter)?

Multicategory axes don't support transforms. I'm not sure how gracefully things fail though.

OK, that's fine. Nothing more from me - I'll let one of the other folks give this a once-over and the 💃

Transforms + multicategory tracked in -> #3363

alexcjohnson · 2018-12-21T16:17:48Z

+ ax.clearCalc();
+ if(ax.type === 'multicategory') {
+ ax.setupMultiCategory(fullData);
+ }


clearCalc is also used in rangeslider.draw and gl2d/scene2d - do these need setupMultiCategory? Should that just be part of clearCalc for multicategory axes?

clearCalc is also used in rangeslider.draw and gl2d/scene2d - do these need setupMultiCategory?
setupMultiCategory only needs to be called once (here in doCalcdata, to fill in ax._muticatList), so rangeslider don't need it, that's why I didn't put the setupMultiCategory logic in clearCalc. As for gl2d/scene2d, gl2d subplots don't support multicategory axes, so I think we're 👌

archmoj · 2018-12-21T20:19:46Z

💃

etpinard · 2018-12-21T15:57:08Z

fixes#3356 - and made me realised that multicategory are slightly more complicated than first expected.

In brief, we need to (1) scan the data array (2) sort the resulting multi-category list (3) loop over the data arrays again to retrieve their calcdata values.

cc @plotly/plotly_js

- so that we handle cross-trace sorting correctly

alexcjohnson · 2018-12-21T16:14:21Z


 // clear stuff that should recomputed in 'regular' loop
- if(hasCalcTransform) clearAxesCalc(axList);
+ if(hasCalcTransform) setupAxisCategories(axList);


, fullData?

maybe a can of worms, but is there a test for multicat + calctransforms (or non-calc-transforms for that matter)?

Multicategory axes don't support transforms. I'm not sure how gracefully things fail though.

OK, that's fine. Nothing more from me - I'll let one of the other folks give this a once-over and the 💃

Transforms + multicategory tracked in -> #3363

alexcjohnson · 2018-12-21T16:17:48Z

+ ax.clearCalc();
+ if(ax.type === 'multicategory') {
+ ax.setupMultiCategory(fullData);
+ }


clearCalc is also used in rangeslider.draw and gl2d/scene2d - do these need setupMultiCategory? Should that just be part of clearCalc for multicategory axes?

clearCalc is also used in rangeslider.draw and gl2d/scene2d - do these need setupMultiCategory?
setupMultiCategory only needs to be called once (here in doCalcdata, to fill in ax._muticatList), so rangeslider don't need it, that's why I didn't put the setupMultiCategory logic in clearCalc. As for gl2d/scene2d, gl2d subplots don't support multicategory axes, so I think we're 👌

archmoj · 2018-12-21T20:19:46Z

💃

etpinard · 2018-12-21T15:57:08Z

fixes#3356 - and made me realised that multicategory are slightly more complicated than first expected.

In brief, we need to (1) scan the data array (2) sort the resulting multi-category list (3) loop over the data arrays again to retrieve their calcdata values.

cc @plotly/plotly_js

- so that we handle cross-trace sorting correctly

alexcjohnson · 2018-12-21T16:14:21Z


 // clear stuff that should recomputed in 'regular' loop
- if(hasCalcTransform) clearAxesCalc(axList);
+ if(hasCalcTransform) setupAxisCategories(axList);


, fullData?

maybe a can of worms, but is there a test for multicat + calctransforms (or non-calc-transforms for that matter)?

Multicategory axes don't support transforms. I'm not sure how gracefully things fail though.

OK, that's fine. Nothing more from me - I'll let one of the other folks give this a once-over and the 💃

Transforms + multicategory tracked in -> #3363

alexcjohnson · 2018-12-21T16:17:48Z

+ ax.clearCalc();
+ if(ax.type === 'multicategory') {
+ ax.setupMultiCategory(fullData);
+ }


clearCalc is also used in rangeslider.draw and gl2d/scene2d - do these need setupMultiCategory? Should that just be part of clearCalc for multicategory axes?

clearCalc is also used in rangeslider.draw and gl2d/scene2d - do these need setupMultiCategory?
setupMultiCategory only needs to be called once (here in doCalcdata, to fill in ax._muticatList), so rangeslider don't need it, that's why I didn't put the setupMultiCategory logic in clearCalc. As for gl2d/scene2d, gl2d subplots don't support multicategory axes, so I think we're 👌

archmoj · 2018-12-21T20:19:46Z

💃

etpinard · 2018-12-21T15:57:08Z

fixes#3356 - and made me realised that multicategory are slightly more complicated than first expected.

In brief, we need to (1) scan the data array (2) sort the resulting multi-category list (3) loop over the data arrays again to retrieve their calcdata values.

cc @plotly/plotly_js

- so that we handle cross-trace sorting correctly

alexcjohnson · 2018-12-21T16:14:21Z


 // clear stuff that should recomputed in 'regular' loop
- if(hasCalcTransform) clearAxesCalc(axList);
+ if(hasCalcTransform) setupAxisCategories(axList);


, fullData?

maybe a can of worms, but is there a test for multicat + calctransforms (or non-calc-transforms for that matter)?

Multicategory axes don't support transforms. I'm not sure how gracefully things fail though.

OK, that's fine. Nothing more from me - I'll let one of the other folks give this a once-over and the 💃

Transforms + multicategory tracked in -> #3363

alexcjohnson · 2018-12-21T16:17:48Z

+ ax.clearCalc();
+ if(ax.type === 'multicategory') {
+ ax.setupMultiCategory(fullData);
+ }


clearCalc is also used in rangeslider.draw and gl2d/scene2d - do these need setupMultiCategory? Should that just be part of clearCalc for multicategory axes?

clearCalc is also used in rangeslider.draw and gl2d/scene2d - do these need setupMultiCategory?
setupMultiCategory only needs to be called once (here in doCalcdata, to fill in ax._muticatList), so rangeslider don't need it, that's why I didn't put the setupMultiCategory logic in clearCalc. As for gl2d/scene2d, gl2d subplots don't support multicategory axes, so I think we're 👌

archmoj · 2018-12-21T20:19:46Z

💃

etpinard · 2018-12-21T15:57:08Z

fixes#3356 - and made me realised that multicategory are slightly more complicated than first expected.

In brief, we need to (1) scan the data array (2) sort the resulting multi-category list (3) loop over the data arrays again to retrieve their calcdata values.

cc @plotly/plotly_js

- so that we handle cross-trace sorting correctly

alexcjohnson · 2018-12-21T16:14:21Z


 // clear stuff that should recomputed in 'regular' loop
- if(hasCalcTransform) clearAxesCalc(axList);
+ if(hasCalcTransform) setupAxisCategories(axList);


, fullData?

maybe a can of worms, but is there a test for multicat + calctransforms (or non-calc-transforms for that matter)?

Multicategory axes don't support transforms. I'm not sure how gracefully things fail though.

OK, that's fine. Nothing more from me - I'll let one of the other folks give this a once-over and the 💃

Transforms + multicategory tracked in -> #3363

alexcjohnson · 2018-12-21T16:17:48Z

+ ax.clearCalc();
+ if(ax.type === 'multicategory') {
+ ax.setupMultiCategory(fullData);
+ }


clearCalc is also used in rangeslider.draw and gl2d/scene2d - do these need setupMultiCategory? Should that just be part of clearCalc for multicategory axes?

clearCalc is also used in rangeslider.draw and gl2d/scene2d - do these need setupMultiCategory?
setupMultiCategory only needs to be called once (here in doCalcdata, to fill in ax._muticatList), so rangeslider don't need it, that's why I didn't put the setupMultiCategory logic in clearCalc. As for gl2d/scene2d, gl2d subplots don't support multicategory axes, so I think we're 👌

archmoj · 2018-12-21T20:19:46Z

💃

etpinard added 3 commits December 21, 2018 09:39

fix multicategory ordering after sorting
d3d7b47

mv multicat scan+sorting before ax.makeCalcdata
7e4c85a
- so that we handle cross-trace sorting correctly

rename clearAxesCalc -> setupAxisCategories
5b490f7

etpinard added bug something broken status: reviewable labels Dec 21, 2018

alexcjohnson reviewed Dec 21, 2018
View reviewed changes

fixup setupAxisCategories call signature
583a8f3

archmoj reviewed Dec 21, 2018
View reviewed changes

Comment threadtest/image/mocks/multicategory-sorting.json

mojtaba-proof multicategory sorting mock
96edd72

etpinard merged commit 997e720 into masterDec 21, 2018

etpinard deleted the multicat-fixes branch December 21, 2018 20:20

jonmmease mentioned this pull request Apr 4, 2019
Multi-category sorting misbehaves with missing categories #3723
Open

etpinard added 3 commits December 21, 2018 09:39

fix multicategory ordering after sorting
d3d7b47

mv multicat scan+sorting before ax.makeCalcdata
7e4c85a
- so that we handle cross-trace sorting correctly

rename clearAxesCalc -> setupAxisCategories
5b490f7

etpinard added bug something broken status: reviewable labels Dec 21, 2018

alexcjohnson reviewed Dec 21, 2018
View reviewed changes

fixup setupAxisCategories call signature
583a8f3

archmoj reviewed Dec 21, 2018
View reviewed changes

Comment threadtest/image/mocks/multicategory-sorting.json

mojtaba-proof multicategory sorting mock
96edd72

etpinard merged commit 997e720 into masterDec 21, 2018

etpinard deleted the multicat-fixes branch December 21, 2018 20:20

jonmmease mentioned this pull request Apr 4, 2019
Multi-category sorting misbehaves with missing categories #3723
Open

etpinard added 3 commits December 21, 2018 09:39

fix multicategory ordering after sorting
d3d7b47

mv multicat scan+sorting before ax.makeCalcdata
7e4c85a
- so that we handle cross-trace sorting correctly

rename clearAxesCalc -> setupAxisCategories
5b490f7

etpinard added bug something broken status: reviewable labels Dec 21, 2018

alexcjohnson reviewed Dec 21, 2018
View reviewed changes

fixup setupAxisCategories call signature
583a8f3

archmoj reviewed Dec 21, 2018
View reviewed changes

Comment threadtest/image/mocks/multicategory-sorting.json

mojtaba-proof multicategory sorting mock
96edd72

etpinard merged commit 997e720 into masterDec 21, 2018

etpinard deleted the multicat-fixes branch December 21, 2018 20:20

jonmmease mentioned this pull request Apr 4, 2019
Multi-category sorting misbehaves with missing categories #3723
Open

etpinard added 3 commits December 21, 2018 09:39

fix multicategory ordering after sorting
d3d7b47

mv multicat scan+sorting before ax.makeCalcdata
7e4c85a
- so that we handle cross-trace sorting correctly

rename clearAxesCalc -> setupAxisCategories
5b490f7

etpinard added bug something broken status: reviewable labels Dec 21, 2018

alexcjohnson reviewed Dec 21, 2018
View reviewed changes

fixup setupAxisCategories call signature
583a8f3

archmoj reviewed Dec 21, 2018
View reviewed changes

Comment threadtest/image/mocks/multicategory-sorting.json

mojtaba-proof multicategory sorting mock
96edd72

etpinard merged commit 997e720 into masterDec 21, 2018

etpinard deleted the multicat-fixes branch December 21, 2018 20:20

jonmmease mentioned this pull request Apr 4, 2019
Multi-category sorting misbehaves with missing categories #3723
Open

etpinard added 3 commits December 21, 2018 09:39

fix multicategory ordering after sorting
d3d7b47

mv multicat scan+sorting before ax.makeCalcdata
7e4c85a
- so that we handle cross-trace sorting correctly

rename clearAxesCalc -> setupAxisCategories
5b490f7

etpinard added bug something broken status: reviewable labels Dec 21, 2018

alexcjohnson reviewed Dec 21, 2018
View reviewed changes

fixup setupAxisCategories call signature
583a8f3

archmoj reviewed Dec 21, 2018
View reviewed changes

Comment threadtest/image/mocks/multicategory-sorting.json

mojtaba-proof multicategory sorting mock
96edd72

etpinard merged commit 997e720 into masterDec 21, 2018

etpinard deleted the multicat-fixes branch December 21, 2018 20:20

jonmmease mentioned this pull request Apr 4, 2019
Multi-category sorting misbehaves with missing categories #3723
Open

etpinard added 3 commits December 21, 2018 09:39

fix multicategory ordering after sorting
d3d7b47

mv multicat scan+sorting before ax.makeCalcdata
7e4c85a
- so that we handle cross-trace sorting correctly

rename clearAxesCalc -> setupAxisCategories
5b490f7

etpinard added bug something broken status: reviewable labels Dec 21, 2018

alexcjohnson reviewed Dec 21, 2018
View reviewed changes

fixup setupAxisCategories call signature
583a8f3

archmoj reviewed Dec 21, 2018
View reviewed changes

Comment threadtest/image/mocks/multicategory-sorting.json

mojtaba-proof multicategory sorting mock
96edd72

etpinard merged commit 997e720 into masterDec 21, 2018

etpinard deleted the multicat-fixes branch December 21, 2018 20:20

jonmmease mentioned this pull request Apr 4, 2019
Multi-category sorting misbehaves with missing categories #3723
Open

etpinard added 3 commits December 21, 2018 09:39

fix multicategory ordering after sorting
d3d7b47

mv multicat scan+sorting before ax.makeCalcdata
7e4c85a
- so that we handle cross-trace sorting correctly

rename clearAxesCalc -> setupAxisCategories
5b490f7

etpinard added bug something broken status: reviewable labels Dec 21, 2018

alexcjohnson reviewed Dec 21, 2018
View reviewed changes

fixup setupAxisCategories call signature
583a8f3

archmoj reviewed Dec 21, 2018
View reviewed changes

Comment threadtest/image/mocks/multicategory-sorting.json

mojtaba-proof multicategory sorting mock
96edd72

etpinard merged commit 997e720 into masterDec 21, 2018

etpinard deleted the multicat-fixes branch December 21, 2018 20:20

jonmmease mentioned this pull request Apr 4, 2019
Multi-category sorting misbehaves with missing categories #3723
Open

etpinard added 3 commits December 21, 2018 09:39

fix multicategory ordering after sorting
d3d7b47

mv multicat scan+sorting before ax.makeCalcdata
7e4c85a
- so that we handle cross-trace sorting correctly

rename clearAxesCalc -> setupAxisCategories
5b490f7

etpinard added bug something broken status: reviewable labels Dec 21, 2018

alexcjohnson reviewed Dec 21, 2018
View reviewed changes

fixup setupAxisCategories call signature
583a8f3

archmoj reviewed Dec 21, 2018
View reviewed changes

Comment threadtest/image/mocks/multicategory-sorting.json

mojtaba-proof multicategory sorting mock
96edd72

etpinard merged commit 997e720 into masterDec 21, 2018

etpinard deleted the multicat-fixes branch December 21, 2018 20:20

jonmmease mentioned this pull request Apr 4, 2019
Multi-category sorting misbehaves with missing categories #3723
Open

Uh oh!

Uh oh!

Conversation

etpinard commented Dec 21, 2018• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

archmoj commented Dec 21, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Uh oh!

Conversation

etpinard commented Dec 21, 2018• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

archmoj commented Dec 21, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Uh oh!

Conversation

etpinard commented Dec 21, 2018• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

etpinard commented Dec 21, 2018•
edited
Loading

etpinard commented Dec 21, 2018•
edited
Loading

etpinard commented Dec 21, 2018•
edited
Loading

etpinard commented Dec 21, 2018•
edited
Loading