rework matching & scaleanchor so they work together #5287

alexcjohnson · 2020-11-17T05:56:47Z

This PR removes the limitation that any given axis can only be involved in either a matches or a scaleanchor constraint. To ensure consistency of the constraints we create, any given axis can still only supply a single constraint. Specifically, if matches is supplied, scaleanchor will be ignored within that axis. However one axis may specify matches pointing to an axis that specifies scaleanchor to a third axis, and so on.

For example, to create many subplots whose x axes are all identical, y axes are all identical too, and x and y scales match, you would specify a single scaleanchor, typically between x and y on the first subplot but in principle it could be any of the subplots. Then all the other x axes would specify matches to that first x axis, and all the other y axes would specify matches to that first y axis.

Also, as was the case previously, any attribute that creates a circular chain of constraints (given all the attributes we've already processed) will simply be ignored. At best such an attribute would be redundant, and at worst it would be self-contradictory.

Tests still to come, but please give it a try @emmanuelle

resolve 2 TODOs (test matches with rangebreaks, and avoid duplicating autorange calculations across matching axes)
investigate and fix breakages in existing tests
add image tests of scaleanchor + matches
add interaction tests for scaleanchor + matches
test padded autorange across matching axes (existing bug, fixed in this PR, where 2 matching axes with different autoranges combine those autoranges incorrectly if there's pixel padding like large markers)
tests of interaction bugs in axes_scaleanchor-with-matches (existing bugs, fixed in this PR):
- while panning or drag-zooming the top subplot, the data (in the top subplot) move incorrectly
- after panning or zooming the top subplot, the bottom subplot draws the axes correctly but the data incorrectly

match-scale partial matches + scaleanchor partial 2 scale-match even more match-scale moremore

nicolaskruchten · 2020-11-17T13:22:44Z

Exciting! this closes #3539 and closes #5196, correct?

emmanuelle · 2020-11-17T13:30:29Z

Thanks @alexcjohnson ! It seems to work well for our application (see animated gif) : matching subplots have the correct scale anchor. Looking forward to releasing this cool feature soon! :-)

…action fix

archmoj · 2020-11-21T04:20:59Z

Great! 🥇
💃

alexcjohnson · 2020-11-21T17:51:55Z

src/plots/cartesian/autorange.js

+                var ax2 = getFromId(gd, axId2);
+                var extremes2 = concatExtremes(gd, ax2, true);
+                // convert padding on the second axis to the first with lenRatio
+                var lenRatio = ax._length / ax2._length;


Previously we calculated autorange for matched axes incorrectly: we just took the minimum and maximum of all the individually-calculated ranges. This is fine when there's no padding around the edges (pixel padding or the 5% "extrapad") but when we have either of these, the simple combination of results gives too little padding. So what I changed to instead is combining the extremes arrays (sets of min and max items each with its own padding) by converting pixel padding to its equivalent on one axis. Note that this is still compatible with constrain='domain' because, while it shrinks the axes' _length, it does so symmetrically across the whole matching group.

archmoj · 2020-11-21T18:43:59Z

Thanks very much for addressing rangebreaks issue as well as the note on extrapad calculations.
💃
After merging this PR I should apply these changes in #5027.

alexcjohnson · 2020-11-21T19:17:37Z

src/plots/cartesian/constraints.js

+    var domain = ax.domain;
+    if(!domain) {
+        // at this point overlaying axes haven't yet inherited their main domains
+        // TODO: constrain: domain with overlaying axes is likely a bug.


This one I haven't tested, but it certainly seems problematic to have overlaying axes and use the domain (which would apply to all of them) to satisfy a constraint on just one.

alexcjohnson · 2020-11-21T19:25:10Z

src/plots/cartesian/constraints.js

+}
+
+// scales may be numbers or 'x1.3', 'yy4.5' etc to multiply by as-yet-unknown
+// ratios between x and y plot sizes n times


See the mock axes_chain_scaleanchor_matches2 for an extreme example of this - chaining matches and scaleanchor constraints while going back and forth between the x and y directions will compound the subplot aspect ratios. Probably we won't ever see a plot that takes it to that extreme, but you can imagine for example creating an engineering diagram with front, top, and side views - three subplots so six axes, comprising three matching pairs all scaled together - where you start down this path. So I felt it important to make it work.

alexcjohnson · 2020-11-21T19:29:56Z

src/plots/cartesian/dragbox.js

+        links = calcLinks(gd, gd._fullLayout._axisConstraintGroups, xaHash, yaHash, matches);
+        var spConstrained = links.isSubplotConstrained || matches.isSubplotConstrained;
+        editX = ew || spConstrained;
+        editY = ns || spConstrained;


This whole section is badly in need of a rewrite, but I didn't do it here, I just patched it up to work - fixing some existing bugs that are particularly evident in axes_scaleanchor-with-matches. Ideally we would make it much more explicit what's going on, rather than the very indirect actions happening in dragAxList, updateMatchedAxRange, updateSubplots, and ticksAndAnnotations.

alexcjohnson · 2020-11-21T19:36:16Z

test/image/mocks/axes_breaks-matches.json

+        {
+            "mode": "lines+markers",
+            "x": [
+                "2020-05-09",


If you look at the baseline for this mock, you can see a bug in the autorange where the left edge gets no padding. This has nothing to do with matching axes, it'll happen on a single axis with rangebreaks, just because the first point is inside a break.

alexcjohnson · 2020-11-21T19:44:05Z

test/jasmine/tests/cartesian_interact_test.js

                    [['yaxis'], yr0, {noChange: true}]
                ],
                dblclickSubplot: 'xy'
            }, {
                desc: 'drag e on xy',
                drag: ['xy', 'e', 30, 30],
                exp: [
-                    [['xaxis', 'xaxis2', 'xaxis3'], [xr0[0], 1.366], {dragged: true}],
+                    // FIXME On CI we need 1.359 but locally it's 1.317 ??
+                    [['xaxis', 'xaxis2', 'xaxis3'], [xr0[0], 1.359], {dragged: true}],


I don't understand what's going on here - just this test and the next one (the two where you're dragging one end of one or two axes) have different results on CI and locally. All the other tests behave identically in both environments, the test is repeatable in each environment, and it's clear that there is a change that's roughly what it should be in both cases. Anyway I set it so CI passes but that means it fails locally.

FYI most of the other changes here are due to the matched axes autorange fixes.

rework matching & scaleanchor so they work together

42aabe1

match-scale partial matches + scaleanchor partial 2 scale-match even more match-scale moremore

archmoj added status: has TODOs type: bug type: feature labels Nov 17, 2020

archmoj added this to the v1.58.0 milestone Nov 17, 2020

alexcjohnson added 2 commits November 18, 2020 12:10

compounding aspect ratios edge case test, and minor scale/match inter…

d76a304

…action fix

baseline images altered by improved matching axes autorange

13acf8a

nicolaskruchten mentioned this pull request Nov 19, 2020

Add facet_col and animation_frame argument to imshow plotly/plotly.py#2746

Merged

alexcjohnson added 2 commits November 20, 2020 17:39

remove autorange duplication for matching axes

d9f4641

fix tests & some edge cases uncovered by them

b707c0f

alexcjohnson mentioned this pull request Nov 20, 2020

Trying matching and scaling #5196

Closed

alexcjohnson added 4 commits November 20, 2020 18:39

fix a few baselines for updated matching autorange

bb70db7

update splom tests

0f5c33c

Merge branch 'master' into match-scale-aj

6d291c9

matches + scale interaction tests

1de111d

alexcjohnson commented Nov 21, 2020

View reviewed changes

alexcjohnson added 2 commits November 21, 2020 12:53

fix matches + rangebreaks, with a mock

f78b46e

remove TODO about rangebreaks - done :)

aaf33d3

archmoj removed the status: has TODOs label Nov 21, 2020

alexcjohnson commented Nov 21, 2020

View reviewed changes

alexcjohnson merged commit f7baf5d into master Nov 21, 2020

alexcjohnson deleted the match-scale-aj branch November 21, 2020 19:47

This was referenced Nov 21, 2020

Rangebreaks autorange bug when first point is inside a break #5298

Open

constrain domain and overlaying axes #5299

Open

Implement ticklabelposition for cartesian subplots and colorbars #5275

Merged

nicolaskruchten mentioned this pull request Nov 26, 2020

"matches" attribute interfering with autorange #5095

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rework matching & scaleanchor so they work together #5287

rework matching & scaleanchor so they work together #5287

alexcjohnson commented Nov 17, 2020 •

edited

nicolaskruchten commented Nov 17, 2020

emmanuelle commented Nov 17, 2020 •

edited

archmoj commented Nov 21, 2020

alexcjohnson Nov 21, 2020

archmoj commented Nov 21, 2020

alexcjohnson Nov 21, 2020

alexcjohnson Nov 21, 2020

alexcjohnson Nov 21, 2020

alexcjohnson Nov 21, 2020

alexcjohnson Nov 21, 2020

alexcjohnson Nov 21, 2020

rework matching & scaleanchor so they work together #5287

rework matching & scaleanchor so they work together #5287

Conversation

alexcjohnson commented Nov 17, 2020 • edited

nicolaskruchten commented Nov 17, 2020

emmanuelle commented Nov 17, 2020 • edited

archmoj commented Nov 21, 2020

alexcjohnson Nov 21, 2020

Choose a reason for hiding this comment

archmoj commented Nov 21, 2020

alexcjohnson Nov 21, 2020

Choose a reason for hiding this comment

alexcjohnson Nov 21, 2020

Choose a reason for hiding this comment

alexcjohnson Nov 21, 2020

Choose a reason for hiding this comment

alexcjohnson Nov 21, 2020

Choose a reason for hiding this comment

alexcjohnson Nov 21, 2020

Choose a reason for hiding this comment

alexcjohnson Nov 21, 2020

Choose a reason for hiding this comment

alexcjohnson commented Nov 17, 2020 •

edited

emmanuelle commented Nov 17, 2020 •

edited