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

Correctly handle data values <= 0 on a log scale #5549

Closed
StevenCHowell opened this issue Dec 13, 2016 · 2 comments · Fixed by #5620
Closed

Correctly handle data values <= 0 on a log scale #5549

StevenCHowell opened this issue Dec 13, 2016 · 2 comments · Fixed by #5620

Comments

@StevenCHowell
Copy link
Contributor

This is a continuation from issue #5389, partially adressed by PR #5477. There persists an issue where negative data is not handled correctly. All data <= 0 should be discarded before generating the plot.

As is, if values = np.linspace(-0.1, 0.9), a JS error complains that it "could not set initial ranges", probably because log(n)forn<=0` is not defined.

@clairetang6
Copy link
Contributor

clairetang6 commented Dec 14, 2016

Taking a quick look at this, the problem is definitely because the computed start for the range is NaN. I remember there was code (that I took out EDIT: this code is actually downstream in the log_ticker and was not taken out.) which was labeled hotfix, where the minimum data value was set to 1 whenever it was < 0. That fix also lead to weird behavior (flipped ranges) because the set minimum of 1 could be greater than all of the other data values. Especially in the log plotting context, it is impossible to guess at a minimum value.

The best thing to do would be to set the minimum data value to the the minimum value that is still > 0. But the min data value is saved into the bounds for each renderer and the ranges only use that and don't access all of the data values.

I want to try out a few things to see what would be the best solution. @bryevdv @StevenCHowell do you think it is worth it to first add back the hotfix? I could make that small PR tonight and then figure out a better way to fix it.

Also, the workaround now would be to supply a range start and end.

@StevenCHowell
Copy link
Contributor Author

StevenCHowell commented Dec 14, 2016

@clairetang6, for the hotfix, setting it to 1 will obviously create other problems (typically my data ranges from 0 to 0.3 ish). As you say, something more like the following would be more appropriate:

x_min = min(x[x > 0])

I don't think there is a rush to patch this, just to have to return to it later. Did you see issue #5550? Perhaps consider this as well while trying things as it is closely related.

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

Successfully merging a pull request may close this issue.

3 participants