Skip to content

scattergeo does not respect datum position in arrayOk attributes #1529

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

Closed
etpinard opened this issue Mar 29, 2017 · 1 comment · Fixed by #1538
Closed

scattergeo does not respect datum position in arrayOk attributes #1529

etpinard opened this issue Mar 29, 2017 · 1 comment · Fixed by #1538
Assignees
Labels
bug something broken

Comments

@etpinard
Copy link
Contributor

For example

Plotly.plot('graph', [{
  type: 'scattergeo',
  lon: [10, null, 40],
  lat: [0, null, 0],
  marker: {
    color: ['red', 'green', 'blue'],
    size: [20, 50, 10]
  }
}])

yields

image

see http://codepen.io/etpinard/pen/QpJGeq


This issues comes from #1004 where we started skipping over items with non-numeric lon and/or lat coordinates.

Post #1519 (comment), I'm having second thoughts about skipping non-numeric items in the calc step though mapbox/calc.js appears to get this right. Ignoring non-numeric items early makes sense for 🐎 as we save a few isNumeric call in hot code paths (e.g. in <>/plot.js), but it does lead to some headaches elsewhere in the code (e.g. those placeholder flags) 😕

@etpinard etpinard added the bug something broken label Mar 29, 2017
@etpinard etpinard self-assigned this Mar 29, 2017
@etpinard etpinard changed the title scattergeo does not datum position in arrayOk attributes scattergeo does not respect datum position in arrayOk attributes Mar 29, 2017
@alexcjohnson
Copy link
Collaborator

if isNumeric is a concern (I'm not sure it is compared to everything else that needs to happen in <>/plot.js, but it wouldn't hurt to optimize this) we should be able to ensure <>/calc.js replaces all non-numeric points with BADNUM and just use !== BADNUM which is about as fast as can be imagined.

Right now scatter uses false for non-numerics but that was an ancient decision... if we standardize on something it should be BADNUM.

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

Successfully merging a pull request may close this issue.

2 participants