-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
number like categories respect layout.autotypenumbers
#5880
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
Meaning even "40" gets interpreted as the string "40", a category named "40".
Basically what I am asking is: what difference does |
Is there a case for supporting what happens currently when the coordinates are numbers? |
Yes, I think we need to continue treating actual numbers as indices, but treat numeric strings as categories. The primary reason for this is fractional indices, if you want to draw a line halfway between two categories for example. Traces don't need this, because the whole point of a categorical trace is that the data points are all positioned on categories. |
This is because SVG paths are always strings, so there is no practical way of indicating a number was meant and not a numeric string when specifying SVG paths (sure they could be wrapped in escaped quotes but not today). So when a number is encountered in an SVG (i.e., it is a numeric string), then it is converted to a number so that it is interpreted as an index into layout.categoriesarray.
This made the dragging "jump" when categories were named numeric strings as the strings were all of a sudden interpreted as indices. Lets see if we can't make the pesky shapes test pass some other way.
Now: |
Unfortunately the drag bug is back...
This keeps the tests passing and the correct behaviour when categories are numeric strings.
good thing there's a test for this (bad thing that that is jasmine's way of running one test)
with numeric string categories.
function getRangePosition(v) { | ||
return isNumeric(v) ? +v : getCategoryIndex(v); | ||
function getRangePositionForCategory(v) { | ||
return (typeof v !== 'string') ? v : getCategoryIndex(v); |
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.
Could you please elaborate why using isNumeric
here is not possible?
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.
isNumeric
returns true
for isNumeric('1')
which we don't want. We want numeric string categories to have to use the indices.
}], | ||
"layout":{ | ||
"xaxis": { | ||
"type": "category", |
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.
Let's adjust formatting for these new mocks.
@@ -0,0 +1,105 @@ | |||
'use strict'; |
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.
Let's move all this into test/jasmine/tests/shapes_test.js
.
{ | ||
"data":[{ | ||
"x": ["3", "1", "5", "2", "4", "6"], | ||
"y": ["6","7","8","2","3","4"] |
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.
What happens if we use numbers here?
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.
For this example, it looks the same, but if you try using numbers in the original issue, you get
which means the coordinates in a shape refer to the index of the category on the axis if the shape coordinates are a number and the referenced axis is categorical. In this case the axes are categorical but the categories are numbers (see how they are spaced evenly on the axis even though the numbers are not evenly spaced)
This is probably surprising to users, but the most surprising thing, which is reported in the referenced issue, is that a numeric string is interpreted as an index into the axis.
For context, this is the code:
<!DOCTYPE html>
<html>
<head>
<meta http-equiv='Content-Type' content='text/html:charset=utf-8' />
<script src="../../build/plotly.js"></script>
</head>
<body>
<div id="tester"></div>
<script>
var layout = {
"xaxis": {
"type": "category",
"categoryarray": [
10, 25, 30, 40, 50, 60, 70
],
},
"shapes": [
{
"type": "rect",
"xref": "x",
"yref": "y",
"x0": 40,
"x1": 60,
"y0": 10,
"y1": 20,
"fillcolor": "rgba(45,180,135,0.3)",
}
]
};
var data = [
{
"x": [ 10, 30, 50, 70],
"y": [ 10, 20, 30, 40],
},
];
TESTER = document.getElementById('tester');
Plotly.newPlot( TESTER, data, layout );
</script>
</body>
</html>
This pull request has been sitting for a while, so I would like to close it as part of our effort to tidy up our public repositories. I've assigned it to myself to keep track of it; I'll wait until 2024-06-17 for someone to say it's still relevant and they'll to take it on, and otherwise I will close it then. Thanks - @gvwilson |
Fixes issue #5767
The gist of the fix is in
getRangePosition
inset_convert.js
.But some discussion is still required:
The interpreting of category names that are numeric strings should follow what
layout.axis.autotypenumbers
says to do but it is still not clear to me why the trace data is and was always drawn correctly but the shape data was not.