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

max_y is not inferred correctly when min_y_from_data=true #822

Open
openjck opened this issue Feb 27, 2018 · 1 comment
Open

max_y is not inferred correctly when min_y_from_data=true #822

openjck opened this issue Feb 27, 2018 · 1 comment
Labels

Comments

@openjck
Copy link
Contributor

openjck commented Feb 27, 2018

Steps to reproduce

Download and open this HTML document

Actual result

Screenshot

Description

There's a lot of empty space above the highest data point. The documentation says that by default, "the value [of max_y] is inferred from the dataset." But this doesn't appear to be happening.

If max_y is manually set to 3.2, the chart looks the way I would expect.

Expected result

The chart should look like this (ignoring the weird y-axis labels as described in #821):

screen shot 2018-02-27 at 6 24 53 pm

@cnwangjie
Copy link
Contributor

I think this may be a bug worth paying attention to.
It is caused by this line

max_val = (max_val < 0) ? max_val + (max_val - max_val * args.inflator) * use_inflator : max_val * (use_inflator ? args.inflator : 1);

in mg_min_max_numerical method.

In the example, the max_val is 3.2 at the beginning. After it multiplied by default inflator (10/9), it becomes 3.555555555555556. So it caused such a result.

If the value of y is bigger, the problem will be more obvious. If you set data as

const exampleData = [
    { x: 1, y: 1003.0 },
    { x: 2, y: 1003.1 },
    { x: 3, y: 1003.2 },
];

will cause the following case. (The max_val becomes 1114.6666666666667)

image

And the following case may be what we would rather see.

image

However, for most situations will use_inflator to be enabled. So I think we should make some changes in order to avoid appearing frequently.

I thought it might be possible to multiply inflator by difference between the max_val and the min_val. And let max_val add this value. Insteand of directly multiplying the max_val by inflator.

Also, I think the code of this method is too complicated. Maybe some optimization should make it easier to understand.

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

No branches or pull requests

3 participants