Add autotickangles property

[my-tien]

The new autotickangles property can be used to specify a choice of rotation angles for the labels on the x-axis.
If tickangle is set to "auto", it will be set to the first angle in this array that is large enough to prevent label overlap.

Previously, when tickangle was set to "auto", plotly would choose between a hardcoded 30 or 90 degrees which is still the default for autotickangles, so backwards-compatibility is maintained.

[alexcjohnson]

=== 90 doesn't seem like the right condition here anymore. In fact I'm suspicious of this condition (even before this PR, why is it only a 90-degree rotation that can lead to infinite loops? why not a 30-degree rotation?) so I feel like we should get rid of it and just (inside fixLabelOverlaps) require that if there's a prevAngle we only choose one with Math.abs(newAngle) >= Math.abs(prevAngle)

[my-tien]

I tried to insert my change without having to understand the whole file. So I am not sure what exactly would cause an infinite redraw loop. If I remove the entire if-condition (so the code would always do seq.push(fixLabelOverlaps)) and set the angle to >= prevAngle inside fixLabelOverlaps, this will be okay?

[my-tien]

Please check my latest commit for this change

[archmoj]

Interesting PR.
In regards with test failures, please note that several subplots(e.g. polar, gl3d) as well as components (e.g. colorbar) reuse attributes and functions from cartesian. This means when we add a new attribute like this in cartesian we should skip it in other places.
For example if you search for noTicklabelstep in the code you could find places that we didn't want to have ticklabelstep attribute.
Similarly for your new attribute we should pass something noAutotickangles: true and use it in the coerce functions.

[my-tien]

    autotickangles: {
        valType: 'data_array',
        coerceNumber: true,
        dflt: [30, 90],
        editType: 'ticks',
        description: [
            'When `tickangle` is set to *auto*, it will be set to the first',
            'angle in this array that is large enough to prevent label',
            'overlap.'
        ].join(' ')
    },

Could you help me out with the correct definition here (omitting coerceNumber breaks other things)? Because the jasmin test complains:

all attributes should only have compatible options
plot schema
Error: Expected false to be true, 'coerceNumber', Object({ valType: 'data_array', coerceNumber: true, dflt: [ 30, 90 ], editType: 'ticks', description: 'When tickangle is set to auto, it will be set to the first angle in this array that is large enough to prevent label overlap.' }).

[archmoj]

    autotickangles: {
        valType: 'data_array',
        coerceNumber: true,
        dflt: [30, 90],
        editType: 'ticks',
        description: [
            'When `tickangle` is set to *auto*, it will be set to the first',
            'angle in this array that is large enough to prevent label',
            'overlap.'
        ].join(' ')
    },

Could you help me out with the correct definition here (omitting coerceNumber breaks other things)? Because the jasmin test complains:

all attributes should only have compatible options
plot schema
Error: Expected false to be true, 'coerceNumber', Object({ valType: 'data_array', coerceNumber: true, dflt: [ 30, 90 ], editType: 'ticks', description: 'When tickangle is set to auto, it will be set to the first angle in this array that is large enough to prevent label overlap.' }).

Let's not set coerceNumber: true, for the new attribute as it is not relevant to data_array type.
Instead you should convert string values to numbers in src/plots/cartesian/axes.js.

[my-tien]

Not sure if the last failing test is related to my changes? (test-baselines compare-pixels)

[archmoj]

Not sure if the last failing test is related to my changes? (test-baselines compare-pixels)

Yes. It is the side effect of your PR. Please see the diffs in the ARTIFACTS tab at https://app.circleci.com/pipelines/github/plotly/plotly.js/9395/workflows/fd12b65d-7c84-4420-8194-2568e9111bc3/jobs/205180/artifacts

[my-tien]

Not sure if the last failing test is related to my changes? (test-baselines compare-pixels)

Yes. It is the side effect of your PR. Please see the diffs in the ARTIFACTS tab at https://app.circleci.com/pipelines/github/plotly/plotly.js/9395/workflows/fd12b65d-7c84-4420-8194-2568e9111bc3/jobs/205180/artifacts

Thank you, I introduced the old condition for choosing between 30 and 90 degrees if autotickangles is not provided.

Also, I notice that make-baselines did not regenerate the 10.png, the one I edited to test autotickangles. How do I trigger that?

[archmoj]

Please download new 10.png baseline from https://app.circleci.com/pipelines/github/plotly/plotly.js/9397/workflows/b958b58d-8d09-4c82-90b8-d49dd68a1524/jobs/205218/artifacts and place it at test/image/baselines/ then commit the change.
Thank you!

[archmoj]

Instead of using an angle with "maximum" abs(t) value, shouldn't we pick the angle with "minimum" abs(cos(t))?

[alexcjohnson]

Good point, I hadn't thought about angles beyond [-90, 90]. Would be kind of weird to auto-rotate past vertical but I guess no reason to prohibit it.

[my-tien]

Good point, have changed it to test for minimum absolute cosine.

[alexcjohnson]

I pushed a small commit that makes the behavior what I wanted: when using autotickangles, start with the first angle in the array even if it isn't 0. I also moved a little more of the logic into the defaults step, so that we don't have autotickangles unless we're going to use it (except for the funny log exclusion), which is more in line with our standard practice so that _fullLayout doesn't contain unused attributes.

Then a couple more to fix colorbar ticks and polar ticks (which both thought they were preventing autotickangles, but they weren't and in certain cases they shouldn't!)

@archmoj @my-tien there's still one failing image that I don't quite understand, I can take a look again in the morning, and for some reason I wasn't able to rebuild the schema tonight. Also the same failing jasmine tests as before. But otherwise I think this is good to go from my side!

[archmoj]

Thanks very much @alexcjohnson and @my-tien.
After @alexcjohnson improvements, now on the polar_polygon-grids the tick labels on the magenta graph are rotated 90 degrees!

[archmoj]

Nicely completed.
[ 💃, 👯 , 🕺 ]