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

Using valueRange with Logscale and yRangePad has unexpected results #661

Open
action0jackson opened this issue Sep 23, 2015 · 5 comments
Open
Labels

Comments

@action0jackson
Copy link
Contributor

Please refer to this article: http://stackoverflow.com/questions/32609202/dygraphs-using-valuerange-with-logscale

@danvk danvk added the bug label Oct 20, 2015
@klausw
Copy link
Contributor

klausw commented Jan 4, 2016

I think there's at least two issues here:

https://github.com/danvk/dygraphs/blob/master/src/dygraph.js#L2638

      if (!ypadCompat) {
        if (axis.logscale) {
          var logpad = Math.exp(Math.log(span) * ypad);
          y0 *= logpad;
          y1 /= logpad;
        } else {
          span = y1 - y0;
          y0 -= span * ypad;
          y1 += span * ypad;
        }
      }

First problem is that "axis.logscale" is always false, the axis object doesn't have this property. So the conditional is always false in yRangePad + logscale + valueRange mode, and it inappropriately takes the branch below for a linear scale.

Next problem is that it adjusts in the wrong direction - should be:

          y0 /= logpad;
          y1 *= logpad;

If I make these changes in an interactive session, it looks much more reasonable, though the padding amount isn't quite right - I seem to get just ~3 pixels of padding after requesting 10 pixels.

@klausw
Copy link
Contributor

klausw commented Jan 4, 2016

Here's an updated jsfiddle - the original one didn't work for me: http://jsfiddle.net/hg0nfytr/5/

The default yrange = 0.25005,3 shows the issue, the y axis starts at 1.00e-4. Setting ymin to <= 0.25 breaks the graph entirely.

@action0jackson
Copy link
Contributor Author

Just an fyi I updated the original fiddle to work: http://jsfiddle.net/85f1ga5e/

klausw added a commit to klausw/dygraphs that referenced this issue Mar 1, 2016
Also includes a minor refactor to consolidate duplicated code for
toDataCoord calculations on logscale axes.

Fixes issue danvk#661.
@klausw
Copy link
Contributor

klausw commented Mar 1, 2016

Proposed fix + regression tests in #732 . The padding is now pixel-accurate in logscale.

@klausw
Copy link
Contributor

klausw commented Mar 8, 2016

Fix is merged in master. Looks like I can't close the bug though.

PierrickKoch pushed a commit to PierrickKoch/dygraphs that referenced this issue Sep 15, 2016
Also includes a minor refactor to consolidate duplicated code for
toDataCoord calculations on logscale axes.

Fixes issue danvk#661.
PierrickKoch pushed a commit to PierrickKoch/dygraphs that referenced this issue Sep 22, 2016
Also includes a minor refactor to consolidate duplicated code for
toDataCoord calculations on logscale axes.

Fixes issue danvk#661.
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