-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Add tickmode "proportional" #6827
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
Conversation
- Create tickmode "proportional" - Map fractional tickvals to axis values w/ axis range: - Set ax.(minor?).tickvals to - Do that right before `arrayTicks()` is called - Every instance of `tickmode === "array"` gets `...|| "proportional"` This works well since tickmode "proportional" really just adds a preprocess step to tickmode "array". TODO: - Check if graph is reversed - Find where ticks are redrawn on zoom and make redraw proportional (will probably fix below) - Figure out why ticks not redrawn on double click/home - Add docs in layout
The algo has to set the tickvals property back to original value after spoofing it. This does that.
Re: 6d48a3b "Fix bug by which..." and "redrawing issue":
- Find where ticks are redrawn on zoom (like in "auto") and make redraw proportional (will probably fix below)
- Figure out why ticks not redrawn on double click/home
Issue was that replacing proportional values with actual values causes exponential tickval
increase- each time actual values were to be calcuated, the proportional values used were equal to result of last calculation. This issue is a good argument for adding a whole new property other than tickvals
(tickpropvals
?)- but the relationship with array
is nice for the end user experience.
8743766
to
00e884a
Compare
Alright that's pretty good. One more commit for whatever lint the bot picks up. I'll write tests after I get a response from devs.
Questions for devs:
- Multicategories, should I even try to support it? It actually works kind of well for regular categories, it can help position the ticks.
Tomorrow, a youtube video with results.
Merry Christmas!
image
This strategy currently doesn't work if both major and minor ticks are proportional
because plotly's array-mode conditions in calcTicks
are broken. As of now, calcTicks
cannot actually separate major and minor tick processing into separate loops (like it claims to do through comments and if (major)
) instead processing major and minor ticks in the major loops, and then both again (duplicated) in the minor loops. There are other problems with that: Issue #6828, PR #6829
Plotly does some math on the ticks, sometimes comrparing major and minor values, so we have to store both in their own separate values and then restore them to their attribute at the very end so plotly has them throughout the calculating process.
These tests currently fail but this commit currently doesn't include fundamental bug fixes.
Alright, the testing is parameterized and randomized:
It tests xaxis.tickmode:'proportional'
xaxis.minor.tickmode:'proportional'
yaxis.tickmode:'proportional'
and yaxis.minor.tickmode:'proportional'
in every permutation. Understandably, it's failling as a result of #6828.
All tickval
are randomized, both length
and values.
It runs for category
linear
log
date
type graphs and checks proportions against DOM object properties to verify geometry.
File would obviously be renamed or put into other more relevant location.
I don't understand how this concept would extend to multicategory especially since multicategory is TODO
in arrayTicks
.
Also, not sure how range padding is calculated but it might be convenient if the user were to know what proportion is added by padding so, eg., w/ autorange
, maybe [.05, .95]
would have the intended effect of [0, 1]
.
Interesting PR.
I merged your #6826 pull request.
To simplify the diffs for the reviewers here, I suggest you fetch upstream/master
then after git pull
, merge master
into this branch.
Thank you!
Two suggestions by Alex coming tonight:
- Change Name
- Additional mode to allow generation of ticks along domain w/o
Tomrrow:
3) Test for #2 tomorrow
src/plots/cartesian/axes.js
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you just want one tick I'd put it in the middle. Not that it matters much, this is horrible whatever we choose since you can't tell the scale at all.
src/plots/cartesian/axes.js
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should try to avoid altering real attribute values after the supplyDefaults
step. But you can set an underscored attribute like _tickvals
if you like, then you just need to look for that in arrayTicks
(if you're in one of the domain tickmodes). That should avoid the need to reset it later too, I'd imagine.
Also kudos on figuring out nestedProperty
- but it's overkill here since we know the attribute names, you can just do ax._tickvals = tickVals
etc - or if you feel like code golf, (major ? ax : ax.minor)._tickvals = tickVals
😎
So, I'm happy to write logic + tests for the tick-mode combination but I need it spelled out (and what do we do if they try a combo that doesn't work?- edit: I'll just take a look at how layout_attribute.js
violations handles bad values and probably add the logic in the same place)
To confirm:
Major Tickmode: Auto, Linear allow: Auto # (within major ticks) Linear # (absolute values) Array # (absolute values) Major Tickmode: Array allow: Linear # (absolute values) Array # (absolute values) Major Tickmode: Sync allow: Sync # automatic Major Tickmode: Full Domain allow: Full Domain # (within major ticks) Domain Array # (within major ticks) Major Tickmode: Domain Array allow: Domain Array # (within major ticks)
- Use private ax._mappedTickvals instead - Remove use of Lib.nestedProperty during refactor
- Use a private property
ax._something
instead of overridingax.tickvals
- Remove use of Lib.nestedProperty
- Make some mocks
- Rework math on minor ticks to be relative to major ticks (what do we do if major ticks is
[]
) - Reinspect log math
- Fail if using
domain array
/full domain
in*category
mode (how?) - Maybe take a look at sync while I'm here? (@archmoj if you don't mind)
- Nail down the logic tree for proper major/minor tick combinations
@ayjayt Would you please resolve the conflicts?
Thank you!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Uh oh!
There was an error while loading. Please reload this page.
For #6824:
The basic idea is that we create a mode called 'proportional' which is equivelent to 'array' but values are mapped from proportions before drawn.
(削除) Then again, array doesn't need to have to be recalculated on zoom, so it's not quite so simple. (削除ここまで)[0, .5, 1]
would be[left-most point, middle, right-most point]
.nb: includes my other trivial pull request about meta-charset
TODO: