Skip to content

Conversation

@jnumainville
Copy link
Collaborator

Adds basic docs for map type charts.
Contains a fix for center, which was broken by default for scatter_map, line_map, and density_map.
Also modified default zoom to be 0 instead of 8 (which we did not specify but px sets this). A value of 0 will zoom out to the entire world, which makes more sense if you're not auto calculating the center based on data.

BREAKING CHANGE: scatter_map, line_map, and density_map now have an explicit default zoom of 0 instead of an implicit default zoom of 8

Copy link
Member

@mofojed mofojed left a comment

Choose a reason for hiding this comment

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

Just some minor things to fix up, but some other thoughts/comments:

  • There's a lot of options here we're not showing (projection, map_style, zoom, center, etc). Not saying we need to show all of them, but one example showing a few of those other options would be great to show off the variety of things we can do. The options are already documented which is nice.
  • Maybe we should make a data generator like we have for stocks/fish_market to generate an interesting data set. Doing a wave with the lat/lon is okay, but it's not interesting. Having a data generator that generates something a little more interesting or having a couple options would be nice; e.g. even something that's just focused on one country (e.g. Canada 🍁 ) would be interesting and may drive home some of the differences in the examples. Like right now all the plots kind of start off looking the same, and even between line_map and line_geo, it's not clear the level of detail that is different on these maps since we're zoomed all the way out.

)

# Create the density map plot
# Color is set for better visibility
Copy link
Member

Choose a reason for hiding this comment

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

Colour isn't being set here, this comment is confusing.

Suggested change
# Color is set for better visibility

)

# Create the density map plot
# Color is set for better visibility
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# Color is set for better visibility

# Color is set for better visibility
density_map_plot = dx.density_map(path, lat="Lat", lon="Lon", z="Z")
```

Copy link
Member

Choose a reason for hiding this comment

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

Should we also have an example where the lats/lons are spaced apart a bit more? Right now it just kind of looks like a trailing blob... unsure... I was going to look at the plotly express examples to see what they have, but the link is broken 🤔 : https://plotly.com/python/tile-density-heatmaps/

if center is None:
# Set a default center to prevent px from auto-centering based on data
# which breaks the initial view for map plots since the data may be null initially
# Auto centering on the server side is also a bad idea since the data can change,
Copy link
Member

Choose a reason for hiding this comment

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

Hmm I could us wanting to auto-centre based on the data, when it ticks again... would that be possible? Any way to force Plotly to re-centre when we get a new tick if no centre is specified? I think that might be better than this... either way can file as a separate ticket.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants