MapLibre basemap #2461

birkskyum · 2023-11-30T19:22:19Z

This makes the initial move to maplibre, to get the ball rolling, but it's not complete yet as outlined below. The demo app appear to work just fine. The examples folder doesn't use the local build, but instead a pinned kepler.gl version, so I haven't tried all of them yet.

New styles (and thumbnails) will be needed to replace the "uberdata" ones, since the mapbox styles with their proprietary mapbox:// syntax aren't supported. That is an area I'll need your help with completing. I made a quick darkblue style with maptiler, which is shown in the image below, and carto also has some free ones available (added the dark-matter one).

The GPU layers should work as well now in supported browsers, because maplibre v3 has webgl2 support, and a webgl1 fallback.

ibgreen · 2023-11-30T20:00:53Z

That was quick. I will try to get reviewers eyes on to this asap!

There was a luma.gl patch version mismatch 8.5.20/21, which takes a lot of space in this pr. Is the plan to include deck.gl/luma.gl v9 in this upcoming release?

No, the deck v9 upgrade is going to be the base of the kepler.gl 4.0 release. It is going to require a major lift.

ibgreen · 2023-11-30T20:59:22Z

bindings/kepler.gl-jupyter/js/lib/keplergl/components/app.js

@@ -104,7 +104,7 @@ function App() {
          />
          <link
            rel="stylesheet"
-            href="http://api.tiles.mapbox.com/mapbox-gl-js/v1.1.1/mapbox-gl.css"
+            href="https://unpkg.com/maplibre-gl@^3/dist/maplibre-gl.css"


Q: Have you tested the jupyter integration?

No, in general I had a hard time running the examples and would love some help with that.

How can I test the jupyter bindings locally?

ibgreen · 2023-11-30T21:01:25Z

examples/custom-map-style/src/main.tsx

@@ -22,8 +22,8 @@ import React from 'react';
 import ReactDOM from 'react-dom/client';
 import document from 'global/document';
 import {Provider} from 'react-redux';
-import store from './store';
-import App from './app';
+import store from './store.ts';


Looks like you have some editor that auto sorts imports, package.json etc (or perhaps it just your good habita). But this makes the PR bigger than it has to be and can conflict with other users. Is it possible to avoid these changes? Alternatively get those landed first so the functional change set is smaller.

Sure, I tried to run this example, and got an error that it couldn't find the 'store' and 'app', and it worked when I added the extensions. Will make a PR just for that, and some of the other minor changes that are unrelated to maplibre.

ibgreen · 2023-11-30T21:02:00Z

examples/custom-map-style/src/store.ts

 import keplerGlReducer, {enhanceReduxMiddleware} from '@kepler.gl/reducers';
+import { configureStore } from '@reduxjs/toolkit';
+import { StoreEnhancer } from 'redux';


@macrigiuseppe Need your review on these redux changes.

for context - createStore had a deprecation warning, and a IDE note to use configureStore. It's another thing that can be pulled to another PR if it's easier, because there are many other cases where the createStore is used.

ibgreen · 2023-11-30T21:03:35Z

examples/demo-app/src/cloud-providers/foursquare/foursquare-icon.js

@@ -1,3 +1,23 @@
+// Copyright (c) 2023 Uber Technologies, Inc.


This file was contributed recently long after Uber stopped contributing and transferred ownership to OpenJS foundation. @igorDykhta @macrigiuseppe I assume it is an automated script that adds this, can we disable that?

OpenJS Foundation council clarified we no longer need to carry forward the Uber copyright and instead use "Copyright contributors to the kepler.gl project" (without years)

Opened #2467

ibgreen · 2023-11-30T21:06:28Z

src/constants/src/default-settings.ts

-    colorMode: BASE_MAP_COLOR_MODES.NONE
-  }
+
+  // {


@macrigiuseppe I assume we do need counterparts to all of these in order to load saved maps?

ilyabo · 2023-12-06T13:09:09Z

src/constants/src/default-settings.ts

@@ -315,47 +315,57 @@ export const DEFAULT_MAP_STYLES: {
  {
    id: 'dark',
    label: 'Dark',
-    url: 'mapbox://styles/uberdata/cjoqbbf6l9k302sl96tyvka09',
+    url: 'https://api.maptiler.com/maps/b989b458-e075-497d-9a86-3035fd08bd7c/style.json?key=17oUJVO01Blh1QOfWftj',


Maybe at least the default should be Carto's Dark Matter. It doesn't look that pretty, but it doesn't require setting up an account and creating styles by any particular user. Using a MapTiler account would imply map load limits on their free tier.

Alternatively, we could try to reach out to MapTiler (they say that they love open-source) asking to provide an unlimited key for kepler.

@heshan0131 what do you think?

Reaching out to maptiler seems like a nice thing to do, however in the interest of landing this PR quickly, I would say let's keep things separate and let's just add the unconstrained CARTO styles as defaults for now.

Ideally, users of the kepler.gl framework can easily reconfigure the styles.

ilyabo · 2023-12-06T13:14:21Z

src/components/src/map-container.tsx

@@ -141,15 +141,16 @@ const MAPBOXGL_STYLE_UPDATE = 'style.load';
 const MAPBOXGL_RENDER = 'render';
 const nop = () => {};

-const MapboxLogo = () => (
+const MapLibreLogo = () => (


Maybe we should use react-map-gl's AttributionControl instead. Right now there's no mention of MapTiler or Carto in the attribution when their maps are used. I assume that would mean breaching their licenses.

There should be not a logo but a small attribution text (this is from a deck.gl example ).
.

For some reason this seems to have been lost in the kepler.gl baseline (presumably not an issue with this PR): #2462. If so we could land this and fix that separately. Or fix that first, and rebase to make sure maplibre doesn't clobber it.

If we remove this code in styled-components.tsx (which hides the default attribution):

.maplibregl-ctrl-attrib { display: none; }

and then also remove this in map-container.tsx:

{this.props.primary ? ( <Attribution showMapboxLogo={this.state.showMapboxAttribution} showOsmBasemapAttribution={true} datasetAttributions={datasetAttributions} /> ) : null}

Then the default attribution will show up:

or for dark matter:

Using AttributionControl is actually not required for that. Only if we want to add customAttribution.

Anyways, we should probably rework the attribution rendering, and it can be done in a separate PR.

ibgreen · 2023-12-06T19:31:36Z

@birkskyum can you update the styles to use CARTO's free basemaps? https://github.com/CartoDB/basemap-styles

DarkMatter
DarkMatterNoLabels
Positron
PositronNoLabels
Voyager
VoyagerNoLabels

birkskyum · 2023-12-06T20:12:49Z

@birkskyum can you update the styles to use CARTO's free basemaps? https://github.com/CartoDB/basemap-styles

It's updated now, and will load DarkMatter by default to avoid a white blank canvas.

ibgreen · 2023-12-07T10:24:20Z

@birkskyum Looks like we are starting to get approvals. To prepare for landing, we should get CI to run green.

To fix the DCO issue you should just need to sign your commits before pushing. git commit --signoff
@ilyabo Are you able to provide some advice on what is going wrong with the Node tests?

birkskyum · 2023-12-07T11:37:28Z

@birkskyum Looks like we are starting to get approvals. To prepare for landing, we should get CI to run green.

To fix the DCO issue you should just need to sign your commits before pushing. git commit --signoff

@ilyabo Are you able to provide some advice on what is going wrong with the Node tests?

There are a few things:

A small mock has been added to account for missing worker access in the test environment (potentially jsdom-worker could be used here instead of a mock).
The map styles in the tests should be updated to the carto styles. I'll look into that today. This is mainly in the mapStyleReducer, where tests breaking in relation to topMapStyle / bottomMapStyle. I'm not sure how to resolve them yet.

src/reducers/src/map-style-updaters.ts

birkskyum · 2023-12-07T19:01:36Z

I don't fully understand the remaining "smoothie_the_cat" / bottomMapStyle / topMapStyle tests that are failing. If anyone has experience or input to that, it would be appreciated.

ibgreen · 2023-12-07T19:12:02Z

I don't fully understand the remaining "smoothie_the_cat" / bottomMapStyle / topMapStyle tests that are failing. If anyone has experience or input to that, it would be appreciated.

@igorDykhta can you look at these?

birkskyum · 2023-12-07T19:23:40Z

Interesting, locally I only have the 7 tests failing from MAP_STYLE_CHANGE, but on CI there are 26... don't know what causes the difference.

birkskyum · 2023-12-18T12:12:51Z

Rebased. To ease the review have I reduce the number of changes in this PR, by bumping luma.gl separately (#2463), and reverting some changes caused by IDE auto-formatting.

Signed-off-by: Birk Skyum <birk.skyum@pm.me>

ibgreen · 2023-12-18T14:08:38Z

Rebased. To ease the review have I reduce the number of changes in this PR, by bumping luma.gl separately (#2463), and reverting some changes caused by IDE auto-formatting.

@birkskyum Appreciated! @igorDykhta is trying to get some attention on the test failures so that we can land this asap, before we publish kepler.gl 3.0.

Signed-off-by: Ihor Dykhta <dikhta.igor@gmail.com>

igorDykhta · 2023-12-19T20:13:25Z

@birkskyum Style previews are black for me

birkskyum · 2023-12-19T20:14:51Z

@birkskyum Style previews are black for me

yes, in order to give a preview thumbnails have to be generated and linked. the mapbox style service had an endpoint with a thumbnail, so that is a bit different

ibgreen · 2023-12-19T20:18:58Z

I have landed this to make sure it makes it as part of kepler.gl 3.0 release this Thursday. If possible please add any remaining polish in separate PRs asap.

birkskyum · 2023-12-19T20:35:01Z

@ibgreen , The one existing thumbnail (NO_BASEMAP.png) is stored in this bucket:

https://studio-public-data.foursquare.com/statics/keplergl/geodude/NO_BASEMAP.png

I have generated thumbnails for the 6 new styles (400px X 300px). Is it possible to drop them there as well?

Thumbnails.zip

Signed-off-by: Chris Gervang <chris@gervang.com>

igorDykhta · 2023-12-19T23:42:34Z

https://studio-public-data.foursquare.com/statics/keplergl/geodude/NO_BASEMAP.png

@birkskyum Added thumbnails to the CDN.

birkskyum · 2023-12-20T00:03:47Z

@igorDykhta , thanks! there is a pr to apply them here:

add basemap thumbnails #2486

birkskyum force-pushed the maplibre-basemap branch 2 times, most recently from 6eb76ca to ac5a62a Compare November 30, 2023 19:29

birkskyum marked this pull request as ready for review November 30, 2023 19:45

ibgreen requested review from heshan0131, igorDykhta and macrigiuseppe November 30, 2023 20:00

ibgreen reviewed Nov 30, 2023

View reviewed changes

birkskyum mentioned this pull request Nov 30, 2023

pin luma.gl version to 8.5.21, to avoid mismatch #2463

Merged

ibgreen approved these changes Dec 5, 2023

View reviewed changes

ibgreen requested review from ilyabo and removed request for macrigiuseppe December 5, 2023 21:04

ilyabo reviewed Dec 6, 2023

View reviewed changes

ilyabo approved these changes Dec 7, 2023

View reviewed changes

birkskyum force-pushed the maplibre-basemap branch from 4a0d994 to 1890467 Compare December 7, 2023 10:25

macrigiuseppe reviewed Dec 7, 2023

View reviewed changes

src/reducers/src/map-style-updaters.ts Show resolved Hide resolved

birkskyum force-pushed the maplibre-basemap branch 2 times, most recently from fdf3ab0 to 1aa4cd9 Compare December 18, 2023 12:06

birkskyum added 4 commits December 18, 2023 13:13

Work on MapLibre basemap

a714b76

Signed-off-by: Birk Skyum <birk.skyum@pm.me>

don't update to redux toolkit in this PR

fa16840

Signed-off-by: Birk Skyum <birk.skyum@pm.me>

Use open Carto basemap styles

5924541

Signed-off-by: Birk Skyum <birk.skyum@pm.me>

dark-matter default

1f5b0ed

Signed-off-by: Birk Skyum <birk.skyum@pm.me>

birkskyum added 11 commits December 18, 2023 13:13

mock createObjectURL in jest

e8c8e58

Signed-off-by: Birk Skyum <birk.skyum@pm.me>

Mock createObjectURL

fd983cc

Signed-off-by: Birk Skyum <birk.skyum@pm.me>

Update some unit tests to carto styles

00f31e5

Signed-off-by: Birk Skyum <birk.skyum@pm.me>

work on mapStyleReducer tests

c55b456

Signed-off-by: Birk Skyum <birk.skyum@pm.me>

resolve mapStyleReducer test

db8d3cd

Signed-off-by: Birk Skyum <birk.skyum@pm.me>

work on map style unit tests

536809b

Signed-off-by: Birk Skyum <birk.skyum@pm.me>

revert change to defualt styles

2879d7f

Signed-off-by: Birk Skyum <birk.skyum@pm.me>

fix BaseMapStyle type

16c0677

Signed-off-by: Birk Skyum <birk.skyum@pm.me>

fix lockfile

ba68773

Signed-off-by: Birk Skyum <birk.skyum@pm.me>

don't bump luma.gl in this pr

f66e2f9

Signed-off-by: Birk Skyum <birk.skyum@pm.me>

undo auto-sort

056e9d0

Signed-off-by: Birk Skyum <birk.skyum@pm.me>

birkskyum force-pushed the maplibre-basemap branch from b24f71a to 056e9d0 Compare December 18, 2023 12:13

fix tests

6aea09f

Signed-off-by: Ihor Dykhta <dikhta.igor@gmail.com>

igorDykhta approved these changes Dec 19, 2023

View reviewed changes

ibgreen merged commit 09aee38 into keplergl:master Dec 19, 2023
7 checks passed

chrisgervang pushed a commit to chrisgervang/kepler.gl that referenced this pull request Dec 19, 2023

feat: MapLibre basemap (keplergl#2461)

04b8ac8

Signed-off-by: Chris Gervang <chris@gervang.com>

birkskyum mentioned this pull request Dec 20, 2023

add basemap thumbnails #2486

Merged

birkskyum mentioned this pull request Apr 18, 2024

[Bug] Custom Map Style loading failure #2560

Closed

		@@ -1,3 +1,23 @@
		// Copyright (c) 2023 Uber Technologies, Inc.

MapLibre basemap #2461

MapLibre basemap #2461

Conversation

birkskyum commented Nov 30, 2023 • edited Loading

ibgreen commented Nov 30, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

birkskyum Nov 30, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ilyabo Dec 6, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ibgreen commented Dec 6, 2023 • edited Loading

birkskyum commented Dec 6, 2023 • edited Loading

ibgreen commented Dec 7, 2023

birkskyum commented Dec 7, 2023 • edited Loading

birkskyum commented Dec 7, 2023 • edited Loading

ibgreen commented Dec 7, 2023

birkskyum commented Dec 7, 2023 • edited Loading

birkskyum commented Dec 18, 2023 • edited Loading

ibgreen commented Dec 18, 2023

igorDykhta commented Dec 19, 2023

birkskyum commented Dec 19, 2023 • edited Loading

ibgreen commented Dec 19, 2023

birkskyum commented Dec 19, 2023 • edited Loading

igorDykhta commented Dec 19, 2023

birkskyum commented Dec 20, 2023

birkskyum commented Nov 30, 2023 •

edited

Loading

birkskyum Nov 30, 2023 •

edited

Loading

ilyabo Dec 6, 2023 •

edited

Loading

ibgreen commented Dec 6, 2023 •

edited

Loading

birkskyum commented Dec 6, 2023 •

edited

Loading

birkskyum commented Dec 7, 2023 •

edited

Loading

birkskyum commented Dec 7, 2023 •

edited

Loading

birkskyum commented Dec 7, 2023 •

edited

Loading

birkskyum commented Dec 18, 2023 •

edited

Loading

birkskyum commented Dec 19, 2023 •

edited

Loading

birkskyum commented Dec 19, 2023 •

edited

Loading