Skip to content

Fix lakes and rivers geometry on scoped geo subplots - #4048

Merged
etpinard merged 4 commits into
masterfrom
fixup-geo-scope-lake-polygons
Jul 16, 2019
Merged

Fix lakes and rivers geometry on scoped geo subplots#4048
etpinard merged 4 commits into
masterfrom
fixup-geo-scope-lake-polygons

Conversation

@etpinard

@etpinardetpinard commented Jul 15, 2019

Copy link
Copy Markdown
Contributor

fixes#4046 (a regression from #3856) with the help of etpinard/sane-topojson#14

@etpinardetpinard added this to the v1.49.0 milestone Jul 15, 2019
@etpinard

Copy link
Copy Markdown
ContributorAuthor

cc @plotly/plotly_js don't attempt to update https://codepen.io/nicolaskruchten/pen/orRBKb?editors=0010 from #4046 to see the new behaviour. We need to first update https://cdn.plot.ly/africa_110m.json and friends.

@etpinard

Copy link
Copy Markdown
ContributorAuthor

You'll notice that scope: 'europe' is also currently having issues with showlakes:true.

@@ -0,0 +1,264 @@
{

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

This mock currently looks like:

newplot (42)

@etpinardetpinardJul 15, 2019

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Using topojson files from before #3856 fixes the lake polygons, but the scoped rivers were still off, see 7075d13 or diff in 5385673

@archmoj

Copy link
Copy Markdown
Contributor

It is rather difficult to view & compare the before/after images (with aspect-ratio close to 2) on small monitors. @etpinard would you mind splitting the mock into two mocks?

@etpinard

Copy link
Copy Markdown
ContributorAuthor

@archmoj does this help?

Peek 2019-07-15 15-55

@archmoj

Copy link
Copy Markdown
Contributor

@etpinard I cannot swipe over the center (mid-height) of the image.
Anyway that's just fine.

@antoinerg

Copy link
Copy Markdown
Contributor

Thank you @etpinard for the update and the mock

💃

@etpinard

Copy link
Copy Markdown
ContributorAuthor

Is @archmoj ok also with the changes?

Here's the script I used to generate the new mock:

varschema=Plotly.PlotSchema.get()varscopes=schema.layout.layoutAttributes.geo.scope.valuesvarresolutions=[110,50]vartraces=[]varlayout={grid: {rows: scopes.length,columns: resolutions.length},width: 600,height: 1500}vark=0resolutions.forEach((r,i)=>{scopes.forEach((s,j)=>{varid='geo'+(k ? (k+1) : '')traces.push({type: 'scattergeo',geo: id})layout[id]={scope: s,resolution: r,domain: {column: i,row: j},landcolor: "brown",showland: true,lakecolor: "blue",showlakes: true,rivercolor: "blue",showrivers: true}k++})})Plotly.newPlot(gd,traces,layout,{scrollZoom:false})

It should be easy to transpose (i,j) -> (j,i), if that makes it easier to review for you.

@archmoj

Copy link
Copy Markdown
Contributor

💃

@etpinard
etpinard merged commit 218a919 into masterJul 16, 2019
@etpinard
etpinard deleted the fixup-geo-scope-lake-polygons branch July 16, 2019 13:51
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugsomething broken

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Countries missing in scope=africa

3 participants

@etpinard@archmoj@antoinerg