-
Notifications
You must be signed in to change notification settings - Fork 39
feat: Create richer Basemap
class and deprecate basemap_style
arg
#935
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
base: main
Are you sure you want to change the base?
Conversation
…ithout a basemap yet)
|
||
|
||
class MaplibreBasemap(BaseWidget): | ||
"""A MapLibre GL JS basemap.""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Improve this docstring
] = "reverse-controlled", | ||
style: str | CartoStyle = CartoStyle.PositronNoLabels, | ||
) -> None: | ||
"""Create a MapLibre GL JS basemap.""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
todo: improve this docstring
# lonboard.basemap | ||
|
||
::: lonboard.basemap.CartoBasemap | ||
::: lonboard.basemap.CartoStyle |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to add MaplibreBasemap
to the docs here
const [stateCounter, setStateCounter] = useState<Date>(new Date()); | ||
const updateStateCallback = () => setStateCounter(new Date()); | ||
|
||
useEffect(() => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
todo: in the future, we should figure out a way to refactor some of this code out of the top-level App
, because it's pretty ugly having all of this layer model, basemap model, view model, etc, state in the top-level body.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a richer Basemap
class system to replace the simple string-based basemap_style
parameter, enabling better control over map rendering modes and basemap configuration.
- Adds new
MaplibreBasemap
class with configurable rendering modes ("interleaved", "overlaid", "reverse-controlled") - Deprecates
basemap_style
parameter in favor of the newbasemap
parameter - Adds
before_id
parameter to layers for controlling render order in interleaved mode
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
src/renderers/types.ts | Adds OverlayRendererProps type for interleaved rendering support |
src/renderers/overlay.tsx | Updates overlay renderer to accept interleaved prop |
src/model/initialize.ts | Extracts loadModel helper function for loading widget models |
src/model/basemap.ts | Creates new MaplibreBasemapModel class with mode and style properties |
src/model/base-layer.ts | Adds beforeId property to base layer for render order control |
src/index.tsx | Updates main component to use new basemap system and removes deprecated render_mode |
lonboard/types/map.py | Updates type definitions to use new MaplibreBasemap instead of CartoBasemap |
lonboard/traits.py | Removes unnecessary sync tag from trait initialization |
lonboard/basemap.py | Adds MaplibreBasemap class and renames CartoBasemap to CartoStyle |
lonboard/_viz.py | Updates viz function to use new basemap system |
lonboard/_map.py | Implements deprecation warnings and migration for basemap_style |
lonboard/_layer.py | Adds before_id trait to base layer class |
docs/api/basemap.md | Updates documentation to reference CartoStyle instead of CartoBasemap |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
@kylebarron considering it's possible to pass |
Change list
basemap
parameter toMap
. This now manages a class with more information than just the basemap style. In particular, this holds the render mode"interleaved"
,"overlaid"
, or"reverse-controlled"
that was implemented on the JS side in feat: Support two render modes: Standard/deck.gl-first and MapboxOverlay #921basemap_style
parameter toMap
. This is superseded by thestyle
parameter toMaplibreBasemap
CartoBasemap
toCartoStyle
before_id
parameter to the coreLayer
. When passed and the basemap render mode is"interleaved"
,interleaved
prop down to MapboxOverlayrender_mode
from feat: Support two render modes: Standard/deck.gl-first and MapboxOverlay #921Rendering layer interleaved in maplibre layer stack:
TODO:
beforeId
. IfbeforeId
is passed to a layer, try to resolve the basemap style URL to a dict, and then validate that the string value ofbeforeId
exists in that layer stack. Keep a cache of fetched styles so that you're not fetching them repeatedly.beforeId
is set when the basemap style is not"interleaved"
?For #494