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

Miscellaneous geo fixes #3856

Merged
merged 15 commits into from
May 22, 2019
Merged

Miscellaneous geo fixes #3856

merged 15 commits into from
May 22, 2019
+5,303 −51

Conversation

etpinard
Copy link
Contributor

Three geo fixes.

The fix in 68bd47b is the most interesting where I spent most of the last two days trying to bring back my sane-topojson package back to life (a few things have changed in the Natural Earth dataset and the topojson utility since 2016 😝 ). I settled for a hacky but simple patch for now ... and until sane-topojson is back on solid grounds (cc etpinard/sane-topojson#12)

@plotly/plotly_js reviewing this PR commit-by-commit should be straight-forward enough, but please let me know if you'd like a few codepen examples.

etpinard added 3 commits May 9, 2019 11:23
@etpinard
fix #3831 - reset viewInitial stash on *geo.scope* updates
4abc139
@etpinard
resolves #1754 - add "no-need-to-fetch-topojson" logic
b04102b
@etpinard
fix #3779 - filter out lat<0 features in USA-states location search
68bd47b 
This is important as the Natural Earth files
nclude state/provinces from USA, Canada, Australia and Brazil
which have some overlay in their two-letter ids. For example,
'WA' is used for both Washington state and Western Australia.
As subunits from USA and Canada never conflict, filtering out features
south of the equator suffices to fix #3779

A better fix would have us add a "governing unit" properties in subunit features
in the `sane-topojson` package to avoid conflicts.
@etpinard etpinard added bug something broken status: reviewable labels May 10, 2019
@archmoj
Copy link
Contributor

archmoj commented May 13, 2019

@etpinard Thanks for the fixes.
💃

@etpinard
Copy link
Contributor Author

Thanks for the review @archmoj

I'll leave this PR open for now. I'll try to finish up etpinard/sane-topojson#12 and make the fix in 68bd47b less hacky before 1.48.0 is ready.

@etpinard etpinard mentioned this pull request May 16, 2019
Merged
@etpinard etpinard added the feature something new label May 17, 2019
@etpinard
Copy link
Contributor Author

@archmoj I found time to ✅ etpinard/sane-topojson#12

This PR is reviewable again.

@etpinard etpinard added this to the v1.48.0 milestone May 17, 2019
@archmoj
Copy link
Contributor

archmoj commented May 22, 2019

@etpinard It would nice if sane-topojson could generate dists on multiple lines? Then it may be easier to compare the changes e.g. on the GitHub.

@etpinard
Copy link
Contributor Author

etpinard commented May 22, 2019 •
edited
Loading

It would nice if sane-topojson could generate dists on multiple lines? Then it may be easier to compare the changes e.g. on the GitHub.

It could be nice. Feel free to open a new https://github.com/etpinard/sane-topojson/issues

But comparing MB-large json files isn't my thing, I think comparing diffs like in etpinard/sane-topojson@8624ec4 is much more valuable.

@archmoj
Copy link
Contributor

archmoj commented May 22, 2019

Looking good.
Also thanks for sane-topojson upgrades.
💃

@etpinard
Copy link
Contributor Author

Thanks for the review and thanks for spotting the changes over Crimea:

Peek 2019-05-22 15-51

Peek 2019-05-22 15-52

I'll make sure to include a note about that in the changelog.

@etpinard etpinard merged commit 93af5d6 into master May 22, 2019
@etpinard etpinard deleted the misc-geo-fixes branch May 22, 2019 19:53
@etpinard etpinard mentioned this pull request Jun 12, 2019
Merged
etpinard added a commit that referenced this pull request Jul 15, 2019
@etpinard
baseline geo_lakes-and-rivers using pre-#3856 topojson files
7075d13
@etpinard etpinard mentioned this pull request Jul 15, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@archmoj archmoj

Assignees
No one assigned
Labels
bug something broken feature something new
Projects
None yet
Milestone
v1.48.0
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@etpinard @archmoj