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

number like categories respect layout.autotypenumbers #5880

Draft
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

nicholas-esterer
Copy link
Contributor

Fixes issue #5767
The gist of the fix is in getRangePosition in set_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.

@nicholas-esterer
Copy link
Contributor Author

nicholas-esterer commented Aug 5, 2021

Basically what I am asking is: what difference does layout.axis.autotypenumbers make to drawing the shape on a category axis? If autotypenmbers='strict' the categories haven't been converted to numbers if autotypenumbers='convert types' they are strings, but they are still just keys in a dictionary that look up their position. For example, if xaxis.categoryarray=['10','40','20'] orxaxis.categoryarray=[10,40,20] you can still never place a shape at {x0:10.5 etc.}, it just draws that part of the shape in the middle of the plot, which is what happens when it doesn't find the requested category.
But maybe there's a case I'm not thinking about.

@nicholas-esterer
Copy link
Contributor Author

nicholas-esterer commented Aug 9, 2021

Is there a case for supporting what happens currently when the coordinates are numbers?
The current behaviour is: when shapes are drawn on category axes and the coordinates look like numbers (i.e., they are numbers or numeric strings) then the coordinates are interpreted as indices into categoryarray and the points are placed at the categories in those indices.
Note that this behaviour is NOT seen for traces: if the coordinates look like numbers, the matching category is found, be it number or string (i.e., coordinate 1 can match '1'). If the category is not found, it is added to categoryarray at the end.
The question is: for shapes (and other annotations) do we support the indexing behaviour if the coordinates are numbers but the category-matching behaviour if the coordinates are strings? The question is asked because maybe people rely on the old functionality.

@nicholas-esterer nicholas-esterer marked this pull request as draft August 9, 2021 12:37
@nicholas-esterer
Copy link
Contributor Author

@alexcjohnson
Copy link
Collaborator

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.
@nicholas-esterer
Copy link
Contributor Author

Now:
case 1) if a shape's coordinates are specified as strings, even numeric strings, then the matching category is used to position the shape (e.g., if the categories are ['1','2','3'] then '1' matches '1').
case 2) if a shape's coordinates are specified as numbers then the index of the category is used to position the shape (e.g. if the categories are ['1','2','3'] then 1 matches '2').
However, if a shape is specified as a path, then numbers in this string are always interpreted as indices (case 2).
When a shape whose coordinates are specified as numeric strings is dragged, it does not 'jump' due to the coordinates suddenly being interpreted as indices: the coordinates are converted to indices properly so the drag can work smoothly.

function getRangePosition(v) {
return isNumeric(v) ? +v : getCategoryIndex(v);
function getRangePositionForCategory(v) {
return (typeof v !== 'string') ? v : getCategoryIndex(v);
Copy link
Contributor

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?

Copy link
Contributor Author

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",
Copy link
Contributor

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';
Copy link
Contributor

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"]
Copy link
Contributor

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?

Copy link
Contributor Author

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
bugstillthere

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>

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

Successfully merging this pull request may close these issues.

None yet

3 participants