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

Bar chart X axis autorange not working as expected ? #5200

Closed
LoganWlv opened this issue Oct 8, 2020 · 7 comments
Closed

Bar chart X axis autorange not working as expected ? #5200

LoganWlv opened this issue Oct 8, 2020 · 7 comments
Labels
bug something broken

Comments

@LoganWlv
Copy link
Contributor

LoganWlv commented Oct 8, 2020

I have a bart chart with X axis autorange enabled.
X is of type "category" with numbers and a single string to represent not known values.
Somehow the chart is not fully displayed.

Here a minimal example with the issue :

https://codepen.io/loganwlv/pen/mdEdYLW

EDIT: Some extra information can be found there -> stack_overflow
It seems with 7.5  instead of 7.5 on X the graph is displayed as expected

@alexcjohnson
Copy link
Collaborator

Thanks for the report @LoganWlv - this is indeed a bug, the problem is that the range of a category axis is normally expressed in units of the category serial numbers, and as you have 8 categories (0 to 7) and you want to see the whole of each bar (±0.5) the autorange value is [-0.5, 7.5].

The bug is that apparently when we then re-interpret that range to draw the graph, because you have a category of the same name we take 7.5 to mean "the serial number of category 7.5." I imagine we're trying to be flexible and allow you to specify like range: ['apples', 'oranges'] but honestly that's rarely useful because of the ±0.5, or related padding on scatter and other trace types.

@alexcjohnson alexcjohnson added the bug something broken label Oct 11, 2020
@LoganWlv
Copy link
Contributor Author

LoganWlv commented Oct 12, 2020

@alexcjohnson Thanks for the details.

So you confirm that it should be fixed ?
If yes, do you think you could tell me in which file to search for this logic so I may be able to do the fix ?

@alexcjohnson
Copy link
Collaborator

The issue is in converting from "range values" to "calcdata" or "linearized" values, ie the functions ax.r2c or ax.r2l here:

ax.r2c = function(v) {
var index = getCategoryPosition(v);
return index !== undefined ? index : ax.fraction2r(0.5);
};
ax.l2r = ax.c2r = ensureNumber;
ax.r2l = getCategoryPosition;

both of which call out to getCategoryPosition:

function getCategoryPosition(v) {
// d2l/d2c variant that that won't add categories but will also
// allow numbers to be mapped to the linearized axis positions
var index = getCategoryIndex(v);
if(index !== undefined) return index;
if(isNumeric(v)) return +v;
}

I don't think we want to muck with getCategoryPosition, because it's also used by ax.d2r and ax.d2l_noadd and the "d" ("data") values are expected to be category names. Also "range values" are also used by annotations, so in case anyone depends on being able to position an annotation based on the category name I don't think we want to completely drop this ability. So I think we need a new function here that just swaps the precedence, something like:

function getRangePosition(v) {
    return isNumeric(v) ? +v : getCategoryIndex(v);
}

In order to complete the fix, in addition to ensuring this doesn't break any existing tests we'll need a few new ones:

  • Test the case you care about - can be exactly the graph in your codepen, just draw it and check that gd._fullLayout.xaxis._rl is [-0.5, 7.5] rather than [-0.5, 4] - This test should looks similar to (and can be placed next to):

describe('categoryorder', function() {

  • Test that if you add an annotation to a graph that has some numeric categories and some non-numeric categories, providing a non-numeric category as the annotation position will attach it to that category, but providing a numeric category will attach it to the serial number position instead. There actually is a basic test of the latter case:

// you can also use category serial numbers

But that's in the context of testing clicktoshow and doesn't really test where it's positioned. For that purpose we may need to test the bounding box, though those tests are a bit tricky to get working well across platforms. Alternatively we could add a new image test covering these cases - similar to simple_annotation but with a category axis and a few more annotations.

@LoganWlv
Copy link
Contributor Author

Thanks a lot. I'll try to make the fix

@LoganWlv
Copy link
Contributor Author

LoganWlv commented Oct 13, 2020

@alexcjohnson I created a PR.
As you said I did add a unit test to check the computed range + one image test, it is working.

Though in local when running jasmine tests it breaks on multiple tests (with and without the fix). So I am wondering if there is something wrong in my local configuration and if I could mimic the circleCI test pipeline with docker.

Some tests which are broken in local (without the fix)

Chrome 86.0.4240.75 (Mac OS 10.15.7) Animating multiple axes @flaky updates ranges of secondary axes (date + category case) FAILED

Chrome 86.0.4240.75 (Mac OS 10.15.7) axis zoom/pan and main plot zoom updates matching axes no-constrained x-axes matching x-axes subplot case zoombox on xy FAILED
	Error: Expected 1.4770202987361165,2.33875909613175 to be close to 1.494,2.35 xaxis - after zoombox on xy |input



Chrome 86.0.4240.75 (Mac OS 10.15.7) axis zoom/pan and main plot zoom updates matching axes no-constrained x-axes matching x-axes subplot case drag e on xy FAILED
	Error: Expected -0.2464572960551513,1.3391504565336907 to be close to -0.245,1.366 xaxis - after drag e on xy |input

Chrome 86.0.4240.75 (Mac OS 10.15.7) Indicator plot numbers scale down but never back up if domain size is constant FAILED
	Error: Expected '0.6740981304309663' to be close to 0.8, 1, 'should scale down'.

...

@alexcjohnson
Copy link
Collaborator

Great, I'll take a look at your PR!

Don't worry about the local test failures - some of those interaction tests are tricky to get working right, and the @flaky ones sometimes fail anyway.

alexcjohnson added a commit that referenced this issue Oct 13, 2020
…q-category

Fix autorange computation when a category match a range extreme
@alexcjohnson
Copy link
Collaborator

Closed by @LoganWlv in #5211

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something broken
Projects
None yet
Development

No branches or pull requests

2 participants