Add tickmode "proportional" #6827

ayjayt · 2023-12-26T02:33:26Z

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:

- 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.

ayjayt · 2023-12-26T04:28:09Z

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.

ayjayt · 2023-12-26T07:13:51Z

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!

ayjayt · 2023-12-27T04:48:22Z

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.

ayjayt · 2023-12-28T23:54:20Z

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].

archmoj · 2024-01-02T14:33:10Z

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!

ayjayt · 2024-01-02T23:38:48Z

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

ayjayt · 2024-01-04T03:13:46Z

Ok! There will be syntax and publish-dist to do after this- also replacing math.Random() w/ psuedoRandom()

Note
One issue I have is that jasmine is not set up to parameterize tests, which I like a lot. I generally run tests on my features with every possible configuration, or as many as I can loop through.

The issue is that Jasmine tests are staged in a setup function() and an evaluate function(). Communicating the configuration between the setup (which needs it) and the evaluation (which needs it) requires either use of keyword let in the for loop, or the use of a closure function, which means

for (var i =0; i < N; i=++) {
   (function(i){
       // Test code here, several separate functions `then().then().then()` run with N different configurations.
    })(i);
}

It's ugly but the other solutions are using let or using a jasmine package that allows for parameterization, and that felt too involved for me, since it's more of a long-term testing-architecture decision.

So, parameterization is good in testing. Promises + Jasmine + Karma w/o let or const make it hard to do manually. There should be a parameterization library. There is a way to make it all work (moving all loops outside of it/describe). Which is good, because then each parameter can be reported as a separate test, which improves reporting. But that solution is generally unpopular.

…icks

archmoj · 2024-01-12T21:39:23Z

Thanks for the improvements.
Are you going to add one or few mocks displaying new features?
If so, please start your filename(s) with these letters: zz-... i.e. to avoid a CircleCI bug in our current image test system.

ayjayt · 2024-01-12T22:16:01Z

Sure sure, whatever is needed.

alexcjohnson · 2024-01-19T15:06:32Z

src/plots/cartesian/axes.js

@@ -944,9 +952,61 @@ axes.calcTicks = function calcTicks(ax, opts) {
 axes.prepTicks(mockAx, opts);
 }

+ if(mockAx.tickmode === 'full domain') {
+ var nt = mockAx.nticks; // does mockAx have nitkcs?


Suggested change

var nt = mockAx.nticks; // does mockAx have nitkcs?

var nt = mockAx.nticks;

Yep, you ensured that in tick_value_defaults, then (for minor ticks) this gets pulled into mockAx by the extendFlat.

alexcjohnson · 2024-01-19T20:25:30Z

src/plots/cartesian/axes.js

+ var increment = 1 / (nt - 1); // (nt-2) + 1
+ tickVals.push(0);
+ for(var tickIndex = 0; tickIndex < nt - 2; tickIndex++) {
+ tickVals.push((tickIndex + 1) * increment);
+ }
+ tickVals.push(1);


For major ticks this is good.

For minor, with the existing 'tickmode: 'auto' we take the calculated major ticks, divide the intervals by minor.nticks, and draw minor ticks everywhere except where there are already major ticks. Which means with minor.nticks=5 we actually draw 4 minor ticks in each major interval. That's a little confusing compared with what we're doing here with the new 'full domain' major ticks but I don't see a better design so let's keep major as is, and have minor behave in the analogous way: when major and minor are both 'full domain' we should draw minor.nticks - 1 ticks in each major tick interval.

What about other major/minor tickmode combinations? I think it'd be reasonable to restrict the allowed minor.tickmode based on major tickmode so that at least the major and minor ticks always move together:

tickmode in 'auto', 'linear', 'array' -> minor.tickmode also in 'auto', 'linear', 'array'. We currently allow tickmode: 'array', minor.tickmode: 'auto' but this seems like a bug to me as the minor ticks don't (and can't!) evenly divide major tick intervals, and the spacing of minor ticks depends on minor.nticks but in some strange way. @archmoj was this intentional or should we prohibit minor.tickmode: 'auto' with tickmode: 'array'?

tickmode: 'sync' -> minor ticks should match the minor ticks on the sync'd axis, which they currently almost do, but we get some extras so this is a bug (cc @archmoj - try opening the new_tickmode_sync mock and calling Plotly.relayout(gd,{'yaxis2.minor.ticks':'outside'}) - you'll see the correct minor ticks plus 11 extras, some of which coincide with the expected ticks, some don't, you can see them all if you drag only one of the y axes). Seems to me you should be able to set the visibility and appearance of minor ticks/grid on this axis, but their positions should be sync'd to the positions of minor ticks on the main axis

tickmode in 'full domain', 'domain array' -> minor.tickmode also in 'full domain', 'domain array'. But per my comment above I think with tickmode: 'domain array' we should only accept minor.tickmode: 'domain array' too, or no minor ticks.

I'll take a look at getting minor ticks spaced out between major ticks. Also since I'm working on it I'll take a look at some of the other stuff.

Does plotly throw/raise errors on invalid combinations, or is silent?

alexcjohnson · 2024-01-19T20:36:33Z

src/plots/cartesian/axes.js

+ if(nt === undefined) nt = 0;
+ if(nt === 0) {
+ // pass


For tickmode: 'auto', nticks: 0 or undefined is "super automatic":

plotly.js/src/plots/cartesian/axes.js

Lines 671 to 683 in 620a56b

if(!nt) {

if(ax.type === 'category' || ax.type === 'multicategory') {

minPx = ax.tickfont ? Lib.bigFont(ax.tickfont.size || 12) : 15;

nt = ax._length / minPx;

} else {

minPx = ax._id.charAt(0) === 'y' ? 40 : 80;

nt = Lib.constrain(ax._length / minPx, 4, 9) + 1;

}

// radial axes span half their domain,

// multiply nticks value by two to get correct number of auto ticks.

if(ax._name === 'radialaxis') nt *= 2;

}

ie ending up somewhere between 5 and 10 depending on the graph size. For full domain I wouldn't want us dividing the axis in 7ths though, so maybe just set it to 5 or something? If you really want no ticks just hide them 😉

This also reminds me: the new tickmodes should not be allowed at all on category or multicategory axes.

Silent fail or throw error if they tried to put tickmode in category or multicategory?

alexcjohnson · 2024-01-19T20:41:03Z

src/plots/cartesian/axes.js

+ if(nt === 0) {
+ // pass
+ } else if(nt === 1) {
+ tickVals = [0];


Suggested change

tickVals = [0];

tickVals = [0.5];

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.

alexcjohnson · 2024-01-19T20:57:55Z

src/plots/cartesian/axes.js

+ Lib.nestedProperty(ax, 'tickvals').set(tickVals);
+ } else {
+ Lib.nestedProperty(ax.minor, 'tickvals').set(tickVals);


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 😎

ayjayt · 2024-01-24T21:22:11Z

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

ayjayt · 2024-01-25T02:18:59Z

Use a private property ax._something instead of overriding ax.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 added 2 commits December 25, 2023 22:42

Fix bug by which tickvals forever expands

6d48a3b

The algo has to set the tickvals property back to original value after spoofing it. This does that.

Make tickmode proportional calculate reversed axes

00e884a

ayjayt force-pushed the pikul-proportional-ticks branch from 8743766 to 00e884a Compare December 26, 2023 04:57

ayjayt added 4 commits December 26, 2023 02:20

Use lib.simpleMap(); Refactor var names.

931199e

Add log math to tickmode proportional

fced9ce

Remove console debug messages

b2abf08

Add description in layout_attributes.js

910163e

ayjayt added 2 commits December 26, 2023 04:18

Fix linter errors

e21af96

update plot-scheme diff

ddfaad7

ayjayt mentioned this pull request Dec 27, 2023

Add switch to specify major/minor in axes.arrayTicks() #6829

Merged

ayjayt added 5 commits December 27, 2023 05:06

Remove _isSet flag from tickVals before resetting

5d7b1b9

Add parameterized tests:

db1f82e

These tests currently fail but this commit currently doesn't include fundamental bug fixes.

Fix tests to pass if plotly#6828 is addressed

a2ac023

Clean up commenting/address lint errors.

1fedfc7

Fix a bit more lint

e8f2052

ayjayt marked this pull request as ready for review December 29, 2023 00:21

archmoj added type: feature type: community labels Jan 2, 2024

Merge branch 'master' into pikul-proportional-ticks

c3f590d

ayjayt added 3 commits January 3, 2024 06:24

Change tickmode 'proportional' to 'domain array'

7e93370

Add tickmode 'full domain'

afba178

Set default so full domain doesn't set to auto

aca7a92

ayjayt added 2 commits January 9, 2024 17:55

Restructure parameterization and fix linting

3089c4a

ayjayt force-pushed the pikul-proportional-ticks branch from e4bea21 to 99d336a Compare January 12, 2024 05:30

ayjayt added 6 commits January 12, 2024 00:32

Merge branch 'master' into pikul-proportional-ticks

4476d38

Revert branch to equal master

3502d49

Lint and refactor

09ebcfc

Merge branch 'pikul-proportional-ticks-old' into pikul-proportional-t…

3fe254b

…icks

Revert completely to old tick algo

eb82723

update git-scheme diff

fe6f8d0

archmoj added the status: reviewable label Jan 18, 2024

archmoj requested a review from alexcjohnson January 18, 2024 14:16

alexcjohnson reviewed Jan 19, 2024

View reviewed changes

ayjayt added 3 commits January 24, 2024 18:39

Merge branch 'master' into pikul-proportional-ticks

b3956a0

Remove category test in prep for disallowing

7176049

Refactor as to not override ax.tickvals:

15387ed

- Use private ax._mappedTickvals instead - Remove use of Lib.nestedProperty during refactor

ayjayt added 4 commits January 24, 2024 21:25

Rework some if-then to golf mode (?:)

2ab0b41

Lint

f1a4db8

Make 'auto' to function as 'array' when [] present

ee17af2

Refix incorrect patch ee17af2

93e4bb8

archmoj added the status: has TODOs label Jan 26, 2024

Merge branch 'master' into pikul-proportional-ticks

61efdef

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add tickmode "proportional" #6827

Add tickmode "proportional" #6827

ayjayt commented Dec 26, 2023 •

edited

ayjayt commented Dec 26, 2023 •

edited

ayjayt commented Dec 26, 2023

ayjayt commented Dec 27, 2023

ayjayt commented Dec 28, 2023

archmoj commented Jan 2, 2024

ayjayt commented Jan 2, 2024

ayjayt commented Jan 4, 2024 •

edited

archmoj commented Jan 12, 2024

ayjayt commented Jan 12, 2024

alexcjohnson Jan 19, 2024

alexcjohnson Jan 19, 2024

ayjayt Jan 24, 2024 •

edited

alexcjohnson Jan 19, 2024

ayjayt Jan 24, 2024

alexcjohnson Jan 19, 2024

alexcjohnson Jan 19, 2024

ayjayt commented Jan 24, 2024 •

edited

ayjayt commented Jan 25, 2024 •

edited

	var nt = mockAx.nticks; // does mockAx have nitkcs?
	var nt = mockAx.nticks;

	if(!nt) {
	if(ax.type === 'category' \|\| ax.type === 'multicategory') {
	minPx = ax.tickfont ? Lib.bigFont(ax.tickfont.size \|\| 12) : 15;
	nt = ax._length / minPx;
	} else {
	minPx = ax._id.charAt(0) === 'y' ? 40 : 80;
	nt = Lib.constrain(ax._length / minPx, 4, 9) + 1;
	}

	// radial axes span half their domain,
	// multiply nticks value by two to get correct number of auto ticks.
	if(ax._name === 'radialaxis') nt *= 2;
	}

Add tickmode "proportional" #6827

Are you sure you want to change the base?

Add tickmode "proportional" #6827

Conversation

ayjayt commented Dec 26, 2023 • edited

ayjayt commented Dec 26, 2023 • edited

ayjayt commented Dec 26, 2023

ayjayt commented Dec 27, 2023

ayjayt commented Dec 28, 2023

archmoj commented Jan 2, 2024

ayjayt commented Jan 2, 2024

ayjayt commented Jan 4, 2024 • edited

archmoj commented Jan 12, 2024

ayjayt commented Jan 12, 2024

alexcjohnson Jan 19, 2024

Choose a reason for hiding this comment

alexcjohnson Jan 19, 2024

Choose a reason for hiding this comment

ayjayt Jan 24, 2024 • edited

Choose a reason for hiding this comment

alexcjohnson Jan 19, 2024

Choose a reason for hiding this comment

ayjayt Jan 24, 2024

Choose a reason for hiding this comment

alexcjohnson Jan 19, 2024

Choose a reason for hiding this comment

alexcjohnson Jan 19, 2024

Choose a reason for hiding this comment

ayjayt commented Jan 24, 2024 • edited

ayjayt commented Jan 25, 2024 • edited

ayjayt commented Dec 26, 2023 •

edited

ayjayt commented Dec 26, 2023 •

edited

ayjayt commented Jan 4, 2024 •

edited

ayjayt Jan 24, 2024 •

edited

ayjayt commented Jan 24, 2024 •

edited

ayjayt commented Jan 25, 2024 •

edited