Skip to content

Geo grid fixes #3706

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

Merged
merged 9 commits into from
Apr 5, 2019
Merged

Geo grid fixes #3706

merged 9 commits into from
Apr 5, 2019

Conversation

etpinard
Copy link
Contributor

@etpinard etpinard commented Apr 1, 2019

fixes #2896 (finally) making geo.(lon|lat)axis.tick0 work by computing grid values using Axes.calcTicks.

cc @plotly/plotly_js - I'd like to slip this one in 1.46.0.

@etpinard etpinard added bug something broken status: reviewable labels Apr 1, 2019
@archmoj
Copy link
Contributor

archmoj commented Apr 1, 2019

Great!
Concerning the baselines changes,
@etpinard could you please elaborate why longitudes are changed on polar stereographic projection:
https://github.com/plotly/plotly.js/pull/3706/files?short_path=2b60c83#diff-2b60c83395ec0c848268b4c720e7d73b
Thanks.

@etpinard
Copy link
Contributor Author

etpinard commented Apr 1, 2019

could you please elaborate why longitudes are changed on polar stereographic projection:

We have a slightly different tick algo than d3.geo.graticule.

@archmoj
Copy link
Contributor

archmoj commented Apr 1, 2019

I think the new longitude lines displayed on the polar stereographic projection may be off by one/half pixel or a distance.
For example 20N should cross Hilo island https://www.mapsofworld.com/usa/states/hawaii/lat-long.html.

Old:
old
New:
new

@etpinard
Copy link
Contributor Author

etpinard commented Apr 1, 2019

Sure, there's a diff. Now, is the old baseline correct, or is the new baseline better?

Oh well, I guess I hold this one out of 1.46.0

@etpinard etpinard added this to the v1.47.0 milestone Apr 1, 2019
@archmoj
Copy link
Contributor

archmoj commented Apr 2, 2019

Sure, there's a diff. Now, is the old baseline correct, or is the new baseline better?

Hard to tell. To my eyes the old baseline appear to be slightly more accurate if we compare against Hawaii.

etpinard added 4 commits April 4, 2019 09:35
- so that it matches the current behavior (i.e. where
  tick0 didn't do a thing)
@etpinard
Copy link
Contributor Author

etpinard commented Apr 4, 2019

@archmoj please have a look at fdfca3b where by changing the tick0 dflt, we retrieve something almost identical to the baselines on master.

@archmoj
Copy link
Contributor

archmoj commented Apr 4, 2019

Excellent.
@etpinard could you please increase the width and the height of this mock: test/image/baselines/geo_tick0.png

@archmoj
Copy link
Contributor

archmoj commented Apr 4, 2019

Looks great!
💃 when you wanted to start 1.47.0.

@etpinard
Copy link
Contributor Author

etpinard commented Apr 4, 2019

when you wanted to start 1.47.0.

I'll wait for Monday.

@etpinard
Copy link
Contributor Author

etpinard commented Apr 5, 2019

Ok, no new bug reports from 1.46.0 today, so let's start merging stuff for next week's 1.47.0

@etpinard etpinard merged commit 0b4e549 into master Apr 5, 2019
@etpinard etpinard deleted the geo-grid-fixes branch April 5, 2019 14:58
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 this pull request may close these issues.

tick0 in geo axes is broken
2 participants