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

fixed linechart zero-injection, closes #2102 #2118

Merged
merged 3 commits into from
Dec 8, 2014

Conversation

jthomassie
Copy link

@spenceralger this closes #2102
shelby and I worked on the update, we needed to distinguish between numbers and dates to fix sorting of values.
related to issue #1903 and pr #1968

@@ -19,20 +19,25 @@ define(function (require) {
var flattenedData = flattenDataArray(obj);
var uniqueXValues = {};

var isDate = (obj.ordered && obj.ordered.date);
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd move this var statement to line 20, right after

var flattenedData = flattenDataArray(obj);

@stormpython
Copy link
Contributor

The reason issue #2102 occurred was due to the fix I put in for issue #1903.

The issue in #1903 was that the ordering of vertical bars was incorrect when using a terms aggregation with x axis values that are numbers. For example, when looking at the Count of Documents on a Terms aggregations with the Field of machine.ram or memory.

By default bars should be ordered descending with the x axis values with the highest y axis values appearing first. Since this is the default that elasticsearch returns, we only needed to order the x values by their indexed position in our zero injection code.

However, when x axis values are dates and a Date Histogram aggregation is used, we don't want to sort values by their index position since dates may appear out of order in some cases. So we needed to add code to check to see if the aggregation being used was a Date Histogram and to sort our values by the timestamps instead of by their indexed position.

@jthomassie
Copy link
Author

@stormpython I fixed order of vars and updated comments with more details.
@spenceralger ready for review

@rashidkpc rashidkpc assigned rashidkpc and unassigned spalger Dec 5, 2014
@rashidkpc
Copy link
Contributor

Is there a test for this?

@rashidkpc rashidkpc removed the review label Dec 5, 2014
@rashidkpc rashidkpc assigned jthomassie and unassigned rashidkpc Dec 5, 2014
@rashidkpc rashidkpc mentioned this pull request Dec 8, 2014
rashidkpc added a commit that referenced this pull request Dec 8, 2014
fixed linechart zero-injection, closes #2102
@rashidkpc rashidkpc merged commit 3fa4b94 into elastic:master Dec 8, 2014
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.

Line chart zero-filling broken?
4 participants