Opened 7 years ago

Closed 7 years ago

#15915 closed defect (fixed)

non-indexed column chart are not working if not starting at x=0 and having values greater than the data array length

Reported by: cjolif Owned by: cjolif
Priority: undecided Milestone: 1.8.1
Component: Charting Version: 1.8.0
Keywords: Cc: Mathevet julien
Blocked By: Blocking:

Description (last modified by cjolif)

With #15308 was introduced in Dojo 1.8 the ability to render non-indexed column plots. However if the first value of the series is at x != 0 and having values greater than the data array length the rendering is incorrect.

Attachments (4)

test_15308.html (1.8 KB) - added by cjolif 7 years ago.
test case that does reproduce the issue
bar.png (12.3 KB) - added by Mathevet julien 7 years ago.
fixed bar ?
result.png (9.2 KB) - added by cjolif 7 years ago.
patch_15308.diff (7.1 KB) - added by Mathevet julien 7 years ago.

Download all attachments as: .zip

Change History (18)

Changed 7 years ago by cjolif

Attachment: test_15308.html added

test case that does reproduce the issue

comment:1 Changed 7 years ago by cjolif

Cc: Mathevet julien added
Description: modified (diff)
Summary: non-indexed column chart are not working if not starting at x=0non-indexed column chart are not working if not starting at x=0 and having values greater than the data array length

This comes from the fact the for loop on the values in Columns.js starts with min that is the first x-value in the data but last for getDataLength() which is (if the data array length is less than the maximum value of the data) the number of items in the data not the max - min x-value.

comment:2 Changed 7 years ago by cjolif

Owner: changed from Eugene Lazutkin to cjolif
Status: newassigned

Changed 7 years ago by Mathevet julien

Attachment: bar.png added

fixed bar ?

comment:3 Changed 7 years ago by Mathevet julien

I have differents fix in my dojox source repository. I will try to extract patch that fix this.

Is this the expected graph ? (because I am not sure you also need offset to center bar on x value) fixed bar ?

comment:4 Changed 7 years ago by Mathevet julien

I think it's also due to bar width, see #15477

Could you try to apply fix in #15477, thank's

Last edited 7 years ago by Mathevet julien (previous) (diff)

comment:5 Changed 7 years ago by cjolif

Well bar width is for me a different "issue". With keeping bar width as it is the expected result is the following (ignoring the values on the x-axis):

Last edited 7 years ago by cjolif (previous) (diff)

Changed 7 years ago by cjolif

Attachment: result.png added

comment:6 Changed 7 years ago by Mathevet julien

We don't need to use min and max because we iterate over data elements

comment:7 Changed 7 years ago by cjolif

thanks moogle, I'm going to look at your patch (CLA). Also I suspect the same issue exist on Bar plots as well.

comment:8 Changed 7 years ago by Mathevet julien

Sorry, I updated patch

comment:9 Changed 7 years ago by cjolif

Thanks a lot for the updated patch.

I see two other "issues":

  • StackedColumns/StackedBars now have a getDataLength() method which is never called (and a thus useless maxRunLength variable).
  • we have no more optimization in Columns/Bars chart for a chart that is zoomed. The purpose of the min/max was on an indexed chart that is zoomed to avoid iterating over all the values for nothing but just iterate from the first to the last visible value. This is obviously not working on non indexed as we have seen, but I suspect this is not a good idea to remove that optimization for ones that can benefit from it (and maybe we can make everyone benefit from it).

Changed 7 years ago by Mathevet julien

Attachment: patch_15308.diff added

comment:10 Changed 7 years ago by Mathevet julien

Last update skip data outside axis range.

comment:11 Changed 7 years ago by cjolif

moogle, thanks for your latest patch it seems it wants to accommodate the optimization to non-indexed series as well as to Default (line) plots. At this is meant to be committed in 1.8 branch I want to keep the changes minimal and will take inspiration in your patch to create a simpler version. We should however keep that in mind for 1.9.

comment:12 Changed 7 years ago by cjolif

In [29615]:

refs #15915. Fixes an issue with non-indexed bar/columns plots while keeping indexed plots optimization for zoomed charts. Based on moogle (CLA) patch.

comment:13 Changed 7 years ago by cjolif

Milestone: tbd1.8.1

Fixed in trunk, I will commit in 1.8 branch tomorrow if no regression or problem on this patch are reported by then.

comment:14 Changed 7 years ago by cjolif

Resolution: fixed
Status: assignedclosed

In [29616]:

fixes #15915. Backport to 1.8.x fixes for issue with non-indexed bar/columns plots while keeping indexed plots optimization for zoomed charts. Based on moogle (CLA) patch.

Note: See TracTickets for help on using tickets.