Skip to content

add 'tickformatstops' #1965

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.

Sign up for GitHub

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

Merged
merged 17 commits into from
Oct 16, 2017
Merged

add 'tickformatstops' #1965

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
1baa14a
add 'tickformatstops'
yauhen-kavaliou Aug 17, 2017
f958c3b
change tickformatstops definition and code review fixes
yauhen-kavaliou Aug 23, 2017
a155bd9
change valType
yauhen-kavaliou Aug 24, 2017
be11c03
remove axis attribute duplication
apalchys Aug 24, 2017
a13c29e
fix: add missed semicolon
apalchys Aug 25, 2017
304d96b
add tests for tickformatstops
apalchys Aug 28, 2017
0c9fb2b
add tickformatstop image for test
apalchys Aug 28, 2017
db526d9
Merge branch 'master' into tickformatstops
alexcjohnson Oct 9, 2017
7f1fd41
editTypes for tickformatstops
alexcjohnson Oct 9, 2017
86777f7
make sure the tickformatstops tests actually run as expected
alexcjohnson Oct 9, 2017
531aea8
flatten tickformatstops tests
alexcjohnson Oct 9, 2017
0b48a33
robustify tickformatstops supplydefaults
alexcjohnson Oct 9, 2017
6420044
take the first matching tickformatstop
apalchys Oct 10, 2017
92f7c13
replace Array.prototype.find with for loop and update tests
apalchys Oct 10, 2017
df72531
lint
alexcjohnson Oct 10, 2017
7665b4c
updated tickformatstops baseline
alexcjohnson Oct 10, 2017
0e6747f
merge tickformatstops_test into axes_test
alexcjohnson Oct 12, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/components/colorbar/attributes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ module.exports = {
tickfont: axesAttrs.tickfont,
tickangle: axesAttrs.tickangle,
tickformat: axesAttrs.tickformat,
tickformatstops: axesAttrs.tickformatstops,
tickprefix: axesAttrs.tickprefix,
showtickprefix: axesAttrs.showtickprefix,
ticksuffix: axesAttrs.ticksuffix,
Expand Down
57 changes: 55 additions & 2 deletions src/plots/cartesian/axes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1241,7 +1241,7 @@ function tickTextObj(ax, x, text) {

function formatDate(ax, out, hover, extraPrecision) {
var tr = ax._tickround,
fmt = (hover && ax.hoverformat) || ax.tickformat;
fmt = (hover && ax.hoverformat) || axes.getTickFormat(ax);

if(extraPrecision) {
// second or sub-second precision: extra always shows max digits.
Expand Down Expand Up @@ -1406,7 +1406,7 @@ function numFormat(v, ax, fmtoverride, hover) {
tickRound = ax._tickround,
exponentFormat = fmtoverride || ax.exponentformat || 'B',
exponent = ax._tickexponent,
tickformat = ax.tickformat,
tickformat = axes.getTickFormat(ax),
separatethousands = ax.separatethousands;

// special case for hover: set exponent just for this value, and
Expand Down Expand Up @@ -1507,6 +1507,59 @@ function numFormat(v, ax, fmtoverride, hover) {
return v;
}

axes.getTickFormat = function(ax) {
function convertToMs(dtick) {
return typeof dtick !== 'string' ? dtick : Number(dtick.replace('M', '') * ONEAVGMONTH);
}
function isProperStop(dtick, range, convert) {
var convertFn = convert || function(x) { return x;};
var leftDtick = range[0];
var rightDtick = range[1];
return (!leftDtick || convertFn(leftDtick) <= convertFn(dtick)) &&
(!rightDtick || convertFn(rightDtick) >= convertFn(dtick));
}
function getRangeWidth(range, convert) {
var convertFn = convert || function(x) { return x;};
var left = range[0] || 0;
var right = range[1] || 0;
return Math.abs(convertFn(right) - convertFn(left));
}

var tickstop;
if(ax.tickformatstops && ax.tickformatstops.length > 0) {
switch(ax.type) {
case 'date': {
tickstop = ax.tickformatstops.reduce(function(acc, stop) {
if(!isProperStop(ax.dtick, stop.dtickrange, convertToMs)) {
return acc;
}
if(!acc) {
return stop;
} else {
return getRangeWidth(stop.dtickrange, convertToMs) > getRangeWidth(acc.dtickrange, convertToMs) ? stop : acc;
}
}, null);
break;
}
case 'linear': {
tickstop = ax.tickformatstops.reduce(function(acc, stop) {
if(!isProperStop(ax.dtick, stop.dtickrange)) {
return acc;
}
if(!acc) {
return stop;
} else {
return getRangeWidth(stop.dtickrange) > getRangeWidth(acc.dtickrange) ? stop : acc;
}
}, null);
break;
}
default:
}
}
return tickstop ? tickstop.value : ax.tickformat;
};

axes.subplotMatch = /^x([0-9]*)y([0-9]*)$/;

// getSubplots - extract all combinations of axes we need to make plots for
Expand Down
12 changes: 12 additions & 0 deletions src/plots/cartesian/layout_attributes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -448,6 +448,18 @@ module.exports = {
'*%H~%M~%S.%2f* would display *09~15~23.46*'
].join(' ')
},
tickformatstops: {
valType: 'any',
Copy link
Contributor

Choose a reason for hiding this comment

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

We can do better here.

See for example the layout.slider declaration here. The steps array container is declared using the ✨ special ✨ _isLinkedToArray.

So, this here should look like:

tickformatstops: {
  _isLinkedToArray: 'tickformatstop',

  dtickrange: {
    valType: 'info_array',
    // ... declared just like 'range' above
  },
  values: {
     valType: 'any', 
     // ...
  }
}

arrayOk: true,
role: 'style',
description: [
'Set rules for customizing tickformat on different zoom levels for *date* and',
'*linear axis types. You can specify these rules in following way',
'[{dtickrange: [*min*, *max*], value: *format*}]. Where *min*, *max* - dtick values',
'which describe some zoom level, it is possible to omit *min* or *max* value by passing',
'*null*. *format* - string, exactly as *tickformat*'
].join(' ')
},
hoverformat: {
valType: 'string',
dflt: '',
Expand Down
1 change: 1 addition & 0 deletions src/plots/cartesian/tick_label_defaults.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ module.exports = function handleTickLabelDefaults(containerIn, containerOut, coe

if(axType !== 'category') {
var tickFormat = coerce('tickformat');
coerce('tickformatstops');
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not category?

Copy link
Contributor

Choose a reason for hiding this comment

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

... also, you'll need to a little more work coercing tickformatstop after having addressed https://github.com/plotly/plotly.js/pull/1965/files#r134489590 - see the layout.slider defaults for an example.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why not category?

Since dtick is always 1 for category axis type (at least all my tests show it), I still have no idea how tickformatstops should work for it 🤔 Maybe you have any thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

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

Right, dtickrange wouldn't make sense for category axes. I'm thinking for category axes each tickformatstop item would have:

{
  ticktext: [/* */],
  tickvals: [/* */]
}

just like regular axis-level tickvals and ticktext offering a solution for #1812

The range of each tickformatstop is determined by the tickvals[0] and tickvals[1].

My apologies, I thought I had written that down somewhere before 😏

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No problem, but not sure that I got everything right :)
Let's confirm expected behaviour. For example:

I have the following categories:
['a', 'b', 'c', 'd', 'e']
https://codepen.io/anon/pen/brQwjG

then I define tickformatstops:

tickformatstops: [{
  ticktext: ['a-c'],
  tickvals: ['a', 'c']
}, {
  ticktext: ['d-e'],
  tickvals: ['d', 'e']
}]

and as result I get only 2 ticks:
['a-c', 'd-e']

Is it correct?

@talgalili

Copy link

Choose a reason for hiding this comment

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

Hi @apalchys
This seems reasonable.
Question: does this need to be defined manually, or could it be automatically extracted from the values? For example, if we have 300 categories, that there would be a value setting to show only 30 categories at most. And if we are at a zoom level with many of the categories, it would spread them across the axis.
(I apologies if what I say makes little sense in this context. I more often deal with higher level implementations)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@talgalili I see what you say but not sure how to fit it into the proposed API. I am quite new in Plotly community.
@etpinard maybe you have any idea?
Also is my assumption above correct? If yes, I can start the implementation.

Copy link

Choose a reason for hiding this comment

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

@apalchys checking about this further with @cpsievert - it appears that what I'm looking for is

the ability to have something like nticks to be respected when tickmode='array'

(see the discussion here: talgalili/heatmaply#78 (comment) )

Is there a way the current work would be extended to deal with this?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'll try to think some more about this issue at some point this week. Sorry for the wait.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@etpinard have you had a chance to look into it?

if(!tickFormat && axType !== 'date') {
coerce('showexponent', showAttrDflt);
coerce('exponentformat');
Expand Down
1 change: 1 addition & 0 deletions src/plots/gl3d/layout/axis_attributes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ module.exports = {
exponentformat: axesAttrs.exponentformat,
separatethousands: axesAttrs.separatethousands,
tickformat: axesAttrs.tickformat,
tickformatstops: axesAttrs.tickformatstops,
hoverformat: axesAttrs.hoverformat,
// lines and grids
showline: axesAttrs.showline,
Expand Down
1 change: 1 addition & 0 deletions src/plots/ternary/layout/axis_attributes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ module.exports = {
tickfont: axesAttrs.tickfont,
tickangle: axesAttrs.tickangle,
tickformat: axesAttrs.tickformat,
tickformatstops: axesAttrs.tickformatstops,
hoverformat: axesAttrs.hoverformat,
// lines and grids
showline: extendFlat({}, axesAttrs.showline, {dflt: true}),
Expand Down
12 changes: 12 additions & 0 deletions src/traces/carpet/axis_attributes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,18 @@ module.exports = {
'*%H~%M~%S.%2f* would display *09~15~23.46*'
].join(' ')
},
tickformatstops: {
Copy link
Contributor

Choose a reason for hiding this comment

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

Would you mind DRYing this up by adding

var axesAttrs = require('../../plots/cartesian/layout_attributes')

// ....

     tickformatstops: axesAtttrs.tickformatstops, 

// ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed, thanks.

valType: 'any',
arrayOk: true,
role: 'style',
description: [
'Set rules for customizing tickformat on different zoom levels for *date* and',
'*linear axis types. You can specify these rules in following way',
'[{dtickrange: [*min*, *max*], value: *format*}]. Where *min*, *max* - dtick values',
'which describe some zoom level, it is possible to omit *min* or *max* value by passing',
'*null*. *format* - string, exactly as *tickformat*'
].join(' ')
},
categoryorder: {
valType: 'enumerated',
values: [
Expand Down
2 changes: 1 addition & 1 deletion tasks/util/strict_d3.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ module.exports = transformTools.makeRequireTransform('requireTransform',
var pathOut;

if(pathIn === 'd3' && opts.file !== pathToStrictD3Module) {
pathOut = 'require(\'' + pathToStrictD3Module + '\')';
pathOut = 'require(\'' + pathToStrictD3Module.replace(/\\/g, '/') + '\')';
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need it for windows users. I've updated the code and added comment.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks!

}

if(pathOut) return cb(null, pathOut);
Expand Down