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.

Sign up for GitHub

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

Jump to bottom

Bug fix issue 3224 - avoid NaN values for axis dticks #3233

Merged
merged 7 commits into from
Nov 9, 2018
Merged

Bug fix issue 3224 - avoid NaN values for axis dticks #3233

Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
a8ec02f
prevent NaN to be used in 3d axis ticks
archmoj Nov 8, 2018
fbb4bc9
prevent NaN to be used in 3d axis ticks - fixed syntax
archmoj Nov 8, 2018
965345e
fixing cases where axes._length is NaN
archmoj Nov 8, 2018
9f8f785
clean fix
archmoj Nov 8, 2018
69393b9
added the jasmine test to lock issue 3224
archmoj Nov 9, 2018
3b8b490
Update gl3d_plot_interact_test.js
archmoj Nov 9, 2018
8b670e6
removed -Infinity case of absolute value
archmoj Nov 9, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/plots/cartesian/axes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -755,6 +755,9 @@ axes.autoTicks = function(ax, roughDTick) {
// prevent infinite loops
if(ax.dtick === 0) ax.dtick = 1;

// prevent issue https://github.com/plotly/plotly.js/issues/3224
if(Number.isNaN(ax.dtick)) ax.dtick = 1;
Copy link
Contributor

@etpinard etpinard Nov 8, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Do we know under which circumstances ax.dtick comes through as NaN here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It could simply be triggered for example using gl3d_ibm-plot by panning the graph to outside.
I am wondering now that we may need to replace the condition checks in few other places there too? Noting that isNumeric(NaN)returns false; it skips the adjustment blocks starting with if(!isNumeric(...

Copy link
Contributor

Choose a reason for hiding this comment

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

It could simply be triggered for example using gl3d_ibm-plot by panning the graph to outside.

Hmm. I can't replicate. Can you take a screenshot of

gd._fullLayout.scene.camera.eye

in the console once after that error appears.

I am wondering now that we may need to replace the condition checks in few other places there too?

I'm actually thinking the opposite: we shouldn't be calling Axes.autoTicks in the first place whenever ditck===NaN. But first, I'll like to know how this happens.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm actually thinking the opposite: we shouldn't be calling Axes.autoTicks in the first place whenever ditck===NaN. But first, I'll like to know how this happens.

Agreed. For one thing, dtick=1 would draw 1000 ticks on date or otherwise large-range axes which would bog things down and look ugly. What's roughDTick when we get NaN here, and why?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@etpinard here is a demo

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And this is another example example now without date and logs.

Copy link
Contributor

Choose a reason for hiding this comment

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

Right, this is consistent with what I was thinking: we get those Uncaught ax.dtick error: NaN errors in the console in situations where ticks can't be seen.


// TODO: this is from log axis histograms with autorange off
if(!isNumeric(ax.dtick) && typeof ax.dtick !== 'string') {
var olddtick = ax.dtick;
Expand Down