Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add bounds to range and autorange of cartesian, gl3d and radial axes #6547

Merged
merged 43 commits into from
Aug 24, 2023

Conversation

archmoj
Copy link
Contributor

@archmoj archmoj commented Mar 31, 2023

Resolves #400.
@plotly/plotly_js

@archmoj archmoj changed the title Add autorange bounds to cartesian, gl3d and radial axes Add bounds to range and autorange of cartesian, gl3d and radial axes Apr 3, 2023
@alexcjohnson
Copy link
Collaborator

After chatting with @archmoj last week and playing with this, I think I understand the attributes implemented here:

  • autorange(min|max) - the smallest and largest values that are allowed to be in the results of autoranging this axis. So if these are outside the autorange we calculate they have no effect
  • range(min|max) - the smallest and largest values that are allowed to be in the range EVER. If you try to drag beyond this, or relayout beyond this, we replace either bound that goes outside this range.

Both of these are useful, but I think this is missing an important one, perhaps the first one people will think of in this vein: a specific value to use for one or the other end of autorange, for example "no matter the data, I want the bottom of the range to be 50, but I want Plotly to choose the top of the range." This would almost be a generalization of rangemode: 'tozero', except that even if the data go past zero, the autorange would end at exactly zero.

So I think we need three pairs of attributes, not just two. What if we put them all in a new container:

rangebounds: {
    automin / automax: use these values exactly for autorange
    softmin / softmax: do not let autorange go beyond these values. Ignore if corresponding auto is provided
    hardmin / hardmax: do not let range (auto, explicit, or dynamic) go beyond these values
}

rangebounds.softmin is what you currently have as autorangemin, and rangebounds.hardmin is what you have as rangemin. rangebounds.automin is new behavior.

And two things I notice playing with the PR as it stands today:

  • If one end of autorange is restricted, currently this has no impact on the other end, but it should. For example if there are scatter markers we pad by the marker size plus 5%, but this padding is a smaller data value if the other end of the range is clipped, and bigger if the other end is extended.
  • Make sure the dynamic restrictions work right when you drag. Currently when you drag the trace keeps moving, but the axis stays put. Furthermore, if you drag both ends of the axis (either dragging the center of the axis or panning the whole subplot) and one gets clipped, I think the other end should stop too, rather than starting to zoom at this point.

@nicolaskruchten
Copy link
Contributor

I don't understand "use these values exactly for autorange" ... ? If we say "use these" then what is "auto"?

@nicolaskruchten
Copy link
Contributor

I think a little table like this would help a little:

range min/max (in) rangemode real data min/max auto min/max soft min/max hard min/max range min/max (out)

I assume the idea is that one could set the "in" here by e.g. panning or providing range: [min, max] and then the library would chunk through some calcs that use all of the above as inputs and overwrite range with "out"?

@alexcjohnson
Copy link
Collaborator

The original use case in #400 was to specify one end of the range exactly and have us automatically determine the other end. For that purpose (with the names I'm suggesting) let's say you specify automin=10 - then whether your data range was 1-15 or 14-15, we'd set the range to [10,15]. If your data range was 1-5 we'd have no data to show, but with the minimum constrained to be 10 maybe we'd default to a span of 1, for a final range of [10,11].

I could also imagine saying "I want viewers always to come back to [10,15] when they double click" and setting both automin=1- and automax=15. Then the range is [10,15] regardless of the data. There's no "auto" work for us to do, it's just about controlling the interactive behavior. This is the same as config.doubleClick='reset', but more flexible as it's specified per-axis. Kind of an odd use case, but plausible enough that I'm pleased this set of attributes allows for it.

softmin/softmax would clip the results of autorange to stay within certain bounds. So softmin=0 would be equivalent to rangemode='nonnegative', but it would also allow limiting at any other value as well as limiting the max autorange result.

One thing I'm noticing now is there's currently no equivalent generalization of to rangemode: 'tozero', which basically means "make sure zero is included in the autorange regardless of the data." If we were to add more attributes for that purpose, then rangemode could become just a way to set the defaults for these other attributes, and we ignore it after the supplyDefaults step. One thing to remember though, rangemode: 'tozero' has a very specific bit of behavior when the data get close to zero but don't cross it: It prevents the range from expanding past zero just due to padding:

> Plotly.newPlot(gd,[{y:[0.01,2,3]}],{yaxis:{rangemode:'tozero'}})
> gd.layout.yaxis.range
[0, 3.2047477744807122]  // entirely positive data, padding would expand past zero but tozero prevents this
> Plotly.newPlot(gd,[{y:[-0.01,2,3]}],{yaxis:{rangemode:'tozero'}})
> gd.layout.yaxis.range
[-0.2304777070063694, 3.2204777070063693]  // data on both sides of zero, padding works normally
> Plotly.newPlot(gd,[{y:[-0.01,-2,-3]}],{yaxis:{rangemode:'tozero'}})
> gd.layout.yaxis.range
[-3.2047477744807122, 0]  // totally negative data also won't expand past zero due to padding

An alternative to creating 4 sets of attributes would be one or two sets plus another mode attribute to say how it should behave; I don't like that idea though, because with separate attributes you can specify combinations of behavior: soft and hard limits, an exact setting for one end of the range and a limit on the other end, etc.

Another alternative to 4 attribute pairs would be 4 length-2 arrays, where you can use null for one of the entries if you only want to set one. This is looking more appealing now that we're up to 4 pairs, but three problems with this: (1) only setting one will be the most common situation, so that's a lot of null values cluttering things up, and (2) autorange='reversed' and bounds with one null become ambiguous: is it still [constraintOnMinimum, constraintOnMaximum] or is it [constraintOnRange0, constraintOnRange1]? I think it would need to be the former, otherwise when you flip range you also need to flip all these constraints, but specifying it this way in the first place would feel weird. (3) The rangemode='tozero' generalization doesn't fit the "limit min and/or max" pattern anyway - whatever value(s) you specify need to be included in both the min and the max. So perhaps it could be either a number or a length-2 array (we could accept longer, but we only care about the extrema)

New API suggestion:

With so many it's getting tough to figure out clear, concise names... maybe we put the three autorange-specific ones in a container and leave the hard limits at the top level?

autorangeoptions: {
    min / max: use these values exactly for autorange
    clipmin / clipmax: do not let autorange go beyond these values. Ignore if corresponding min/max is provided
    include: number or [num1, num2], ensure the autorange min <=, and max >=, all of these.
},
rangemin / rangemax: Back to Mojtaba's names, do not let range (auto, explicit, or dynamic) go beyond these values

So then rangemode: 'tozero' would set the default autorangeoptions.include=0 (ignored if there's already a value for include), and rangemode: 'nonnegative' would set the default autorangeoptions.clipmin=0 (likewise ignored if that already exists)

@nicolaskruchten
Copy link
Contributor

I'll have some longer thoughts shortly but here are some assorted intuitions:

  • "auto" to me suggests the output of some automated procedure, which is correct for Plotly.js but I think that it's not obvious to most users when "auto" kicks in. It's basically "whenever range is unset" and "sometimes when double-clicking" right? I also think that most people will not intuit that pan/zoom overwrites range, such that double-clicking would re-invoke auto-range.
  • "min"/"max" have connotations of "minimum actual" i.e. "bottom" or "lower" but also "minimum possible".
  • Taken together, "automin" to me suggests "the smallest possible value that an automated procedure would output" not "the fixed bottom value", that is to say what you call "softmin".
  • I'd also add that as a user, I'd expect that since setting range to null (and [null, null] also?) causes auto-ranging to happen on both values, therefore setting range to [3, null] should cause auto-ranging on the top value by default so I'd recommend having this [3, null] case automatically set the appropriate values of the new attributes if possible!

@nicolaskruchten
Copy link
Contributor

In reaction to your new API proposal, I think that min/clipmin speaks to this ambiguity around "bottom" vs "minimum output of procedure" :)

@nicolaskruchten
Copy link
Contributor

I love the include option!

@nicolaskruchten
Copy link
Contributor

Here's the spreadsheet I'm imagining as the test grid for this feature btw: https://docs.google.com/spreadsheets/d/1DMyGYDSiNoemS78zKjdf_MKKGfSxm3iLQH4r64q-BNg/edit#gid=0

I have some questions around how these interact, and what happens if a min is greater than a max...

@chriddyp
Copy link
Member

With respect the original issue, could a simpler API just be xaxis.range = [5, None] and xaxis.range = [None, 10]? If specified None, then we base it off of the data. That way users don't have to learn a new attribute and it matches the current shape of things.

@alexcjohnson
Copy link
Collaborator

With respect the original issue, could a simpler API just be xaxis.range = [5, null] and xaxis.range = [null, 10]? If specified null, then we base it off of the data. That way users don't have to learn a new attribute and it matches the current shape of things. (AJ edited None -> null, we're in JS here 😉)

I do love the simplicity of this API, and it would let us get rid of autorangeoptions.min/max but we'd keep autorangeoptions.clipmin/max and rangemin/max. Or maybe we call both of these min/maxallowed? In many ways I like that more: consistent names, makes it clear these are limits (rangemin could be taken to mean "the min value is only allowed to be that" rather than "the min is not allowed to go lower than that"). The only thing I don't like is that xaxis.minallowed doesn't explicitly say range so it could be taken to mean "no data to be plotted here can be less than that" - but I think it's better to keep these short and consistent with the names inside autorangeoptions.

What happens though if we later change the data? We write the computed range back to layout.xaxis.range so we've lost the information that only one end should autorange. There's also a question of how to handle reversed axes.

So what if we create 4 new axis.autorange values to go along with these situations:

  • xaxis.range = [5, null] would cause a default xaxis.autorange = 'max' ie "only autorange the max end of the range" but if you want 5 to be the max of a reversed range you can also set xaxis.autorange = 'min reversed'
  • xaxis.range = [null, 10] would cause a default xaxis.autorange = 'min' ie "only autorange the min end of the range" but if you want 10 to be the min of a reversed range you can also set xaxis.autorange = 'max reversed'

Next question is what happens if the viewer doubleclicks, or zooms/pans and then doubleclicks - seems like mostly this should do a full autorange, ie it sets autorange=true (and then by the current default behavior the second doubleclick takes you back to the initial range), but if you wanted autorange still limited or extended on either end you could set autorangeoptions to achieve this. Or if you always want doubleclick to give the initial range you can set config.doubleClick='reset'

So the new API would be:

minallowed: min value allowed in range. Even explicitly setting range:[a,b] can't go past this.
maxallowed: max value allowed in range. Even explicitly setting range:[a,b] can't go past this.
    must be more than minallowed
range: allowed to leave either or both elements `null`, this impacts the default `autorange`
autorange: 'min', 'max', 'min reversed', 'max reversed' as well as existing true, false, 'reversed'
    if 'min' or 'max' is in the value, only autorange that end, use existing `range` entry for the other end
    if range[0] is missing, 'min' is the default and false, 'max', and 'min reversed' are prohibited
    if range[1] is missing, 'max' is the default and false, 'min', and 'max reversed' are prohibited
autorangeoptions: {
    minallowed: do not let min autorange go below this. Ignore if autorange = false, 'max', 'max reversed',
        or if it's <axis.minallowed or >axis.maxallowed
    maxallowed: do not let max autorange go above this. Ignore if autorange = false, 'min', 'min reversed',
        or if it's <axis.minallowed or >axis.maxallowed
    include: number or [num1, num2], ensure the autorange min <= and max >= these values,
        ignore any num outside either set of min/maxallowed constraints
}

@archmoj
Copy link
Contributor Author

archmoj commented Aug 1, 2023

Thanks @archmoj. I retested the previous issue I noticed and it's now working as expected.

One last issue I note is when using this plotly.express example:

import plotly.express as px
df = px.data.iris()

fig = px.scatter(df, x="sepal_width", y="sepal_length", facet_col="species")
fig.update_xaxes(range=[1.5, None])
fig.update_yaxes(range=[3, None])

fig.show()

It doesn't calculate the autorange as I would expect:

image Here's the plotly.js codepen:

https://codepen.io/Liam-Connors/pen/LYXXWJo?editors=1010

Good catch @LiamConnors 🙏
Now fixed in the PR.

@LiamConnors
Copy link
Member

LiamConnors commented Aug 1, 2023

Thanks @archmoj. Looks good to me now. And I don't see any further issues.

dflt: true,
editType: 'axrange',
impliedEdits: {'range[0]': undefined, 'range[1]': undefined},
description: [
'Determines whether or not the range of this axis is',
'computed in relation to the input data.',
'See `rangemode` for more info.',
'If `range` is provided, then `autorange` is set to *false*.'
'If `range` is provided, then `autorange` is set to *false*.',
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this isn't necessarily true anymore 😎

archmoj and others added 2 commits August 2, 2023 13:59
@archmoj archmoj requested a review from alexcjohnson August 18, 2023 16:43
@alexcjohnson
Copy link
Collaborator

@archmoj if I set one end of the range and ask for normal order, the behavior now is perfect AFAICT, ie either of these a doubleclick alternates between the original range and a full autorange, and if I first pan/zoom and then doubleclick, it goes to the original range first, then to a full autorange.:

Plotly.newPlot(gd,[{y:[4,5,6,7]}],{yaxis:{range:[5.5,null]}})
Plotly.newPlot(gd,[{y:[4,5,6,7]}],{yaxis:{range:[null, 5.5]}})

And if I get the reversed setting precisely correct, the behavior is also correct, with axis reversed and doubleclick toggling between initial state and full autorange:

Plotly.newPlot(gd,[{y:[4,5,6,7]}],{yaxis:{range:[null, 5.5], autorange:'max reversed'}})
Plotly.newPlot(gd,[{y:[4,5,6,7]}],{yaxis:{range:[5.5, null], autorange:'min reversed'}})

However, if I instead add autorange: 'reversed' (which I would have expected would be interpreted the same as the two cases above), then a doubleclick does nothing (until I pan/zoom, then doubleclick takes me back to the original as desired, but a second doubleclick should then go to full autorange).

Plotly.newPlot(gd,[{y:[4,5,6,7]}],{yaxis:{range:[null, 5.5], autorange:'reversed'}})
Plotly.newPlot(gd,[{y:[4,5,6,7]}],{yaxis:{range:[5.5, null], autorange:'reversed'}})

Also if I pick the wrong reversed option:

Plotly.newPlot(gd,[{y:[4,5,6,7]}],{yaxis:{range:[null, 5.5], autorange:'max'}})
Plotly.newPlot(gd,[{y:[4,5,6,7]}],{yaxis:{range:[5.5, null], autorange:'min'}})
Plotly.newPlot(gd,[{y:[4,5,6,7]}],{yaxis:{range:[null, 5.5], autorange:'min reversed'}})
Plotly.newPlot(gd,[{y:[4,5,6,7]}],{yaxis:{range:[5.5, null], autorange:'max reversed'}})

I think would expect the autorange setting to be ignored, except maybe for whether it's reversed or not. And that's how it looks initially (the provided end of the range is respected and the other end is autoranged, even though the setting asked for the opposite), but after that a doubleclick takes me to full autorange and then with further doubleclicks we stay there, rather than going back to the initial range.

@archmoj
Copy link
Contributor Author

archmoj commented Aug 23, 2023

Thanks for the review @alexcjohnson.
For those invalid range and autorange combinations now I added validation and tests so that we don't accept range and set autorange to true.

@alexcjohnson
Copy link
Collaborator

Getting much closer! There's still some weird behavior if you set a partial range and autorange: true (it keeps the specified end, but on double-click does not fully autorange) or autorange: 'reversed' (it drops BOTH the reversing and the partial range and gives you full non-reversed autorange).

As I started writing this I was hoping we could allow you to specify a single-ended range along with autorange: 'reversed' and have it infer autorange: 'max reversed' or 'min reversed'. But that's incompatible with the current behavior that if you specify a complete range along with autorange: true where the input range is discarded.

So I think the correct logic must be:

  • First coerce range
  • Then coerce autorange:
    • If no range is given, the only allowed autorange values are [true, 'reversed'], default true
    • If only range[0] is given, autorange may be [true, 'reversed', 'max', 'min reversed'], default 'max'
    • If only range[1] is given, autorange may be [true, 'reversed', 'min', 'max reversed'], default 'min'
    • If both are given, any autorange value is allowed, default false
  • Push the resulting autorange back into the input container
  • Based on the chosen autorange value, either remove the entries from range that we've determined should be autoranged, or ensure the autorange ignores and replaces those values.

What do you think? Does this make sense?

@archmoj
Copy link
Contributor Author

archmoj commented Aug 24, 2023

Getting much closer! There's still some weird behavior if you set a partial range and autorange: true (it keeps the specified end, but on double-click does not fully autorange)

Good catch. Addressed in 30a692e.

@archmoj
Copy link
Contributor Author

archmoj commented Aug 24, 2023

There's still some weird behavior ... autorange: 'reversed' (it drops BOTH the reversing and the partial range and gives you full non-reversed autorange).

I think this is a desired behavior to ignore ambiguous range and autorange combinations and set autorange to true.

Copy link
Collaborator

@alexcjohnson alexcjohnson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough. I still think there are some conditions we could consider bugs and make all of this behavior more self-consistent and simpler, but I'll make a new issue after we merge this.

💃 Nice work on a powerful addition to a very complicated part of our code!

@archmoj archmoj merged commit 0f63668 into master Aug 24, 2023
@archmoj archmoj deleted the autorange-bounds branch August 24, 2023 16:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature something new
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Setting only min or max values for axis and not a range.
5 participants