Skip to content

Commit

Permalink
Fix: st.pydeck_chart with invalid JSON (streamlit#7256)
Browse files Browse the repository at this point in the history
Python's json.dumps does not produce completely valid JSON, which is what Pydeck uses. Using JSON5 to parse instead.
  • Loading branch information
mayagbarnes authored and eric-skydio committed Dec 20, 2023
1 parent a926e76 commit 55ff72a
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 10 deletions.
13 changes: 13 additions & 0 deletions e2e/scripts/st_pydeck_chart.py
Expand Up @@ -12,6 +12,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.

import math
from typing import Any, cast

import numpy as np
Expand Down Expand Up @@ -63,3 +64,15 @@
],
)
)

# Chart w/ invalid JSON - issue #5799.
data = pd.DataFrame({"lng": [-109.037673], "lat": [36.994672], "weight": [math.nan]})
layer = pdk.Layer(
"ScatterplotLayer", data=data, get_position=["lng", "lat"], radius_min_pixels=4
)
deck = pdk.Deck(
layers=[layer],
map_style=pdk.map_styles.CARTO_LIGHT,
tooltip={"text": "weight: {weight}"},
)
st.pydeck_chart(deck, use_container_width=True)
4 changes: 2 additions & 2 deletions e2e/specs/st_pydeck_chart.spec.js
Expand Up @@ -19,10 +19,10 @@ describe("st.pydeck_chart", () => {
cy.loadApp("http://localhost:3000/");
});

it("displays 2 maps", () => {
it("displays 3 maps", () => {
const els = cy.get(".element-container .stDeckGlJsonChart");

els.should("have.length", 2);
els.should("have.length", 3);

els.find("canvas").should("have.css", "height", "500px");
});
Expand Down
Expand Up @@ -15,6 +15,7 @@
*/

import React from "react"
import JSON5 from "json5"
import { render } from "@streamlit/lib/src/test_util"

import { DeckGlJsonChart as DeckGlJsonChartProto } from "@streamlit/lib/src/proto"
Expand Down Expand Up @@ -177,7 +178,7 @@ describe("DeckGlJsonChart element", () => {
const mockJsonParse = jest.fn().mockReturnValue(newJson)

beforeEach(() => {
JSON.parse = mockJsonParse
JSON5.parse = mockJsonParse
})

afterEach(() => {
Expand All @@ -193,26 +194,26 @@ describe("DeckGlJsonChart element", () => {
it(description, () => {
DeckGlJsonChart.getDeckObject(getProps(), originalState)

expect(JSON.parse).not.toHaveBeenCalled()
expect(JSON5.parse).not.toHaveBeenCalled()

DeckGlJsonChart.getDeckObject(getProps(), {
...originalState,
...stateOverride,
})

expect(JSON.parse).toHaveBeenCalledTimes(1)
expect(JSON5.parse).toHaveBeenCalledTimes(1)
})
}

testJsonParsing(
"should call JSON.parse when the element id is different",
"should call JSON5.parse when the element id is different",
{ id: newId }
)
testJsonParsing("should call JSON.parse when FullScreen state changes", {
testJsonParsing("should call JSON5.parse when FullScreen state changes", {
id: mockId,
isFullScreen: true,
})
testJsonParsing("should call JSON.parse when theme state changes", {
testJsonParsing("should call JSON5.parse when theme state changes", {
id: mockId,
isLightTheme: true,
})
Expand Down
Expand Up @@ -16,6 +16,7 @@

import React, { PureComponent, ReactNode } from "react"
import { DeckGL } from "deck.gl"
import JSON5 from "json5"
import isEqual from "lodash/isEqual"
import { MapContext, StaticMap, NavigationControl } from "react-map-gl"
import { withTheme } from "@emotion/react"
Expand Down Expand Up @@ -165,7 +166,7 @@ export class DeckGlJsonChart extends PureComponent<PropsWithHeight, State> {
state.isFullScreen !== currFullScreen ||
state.isLightTheme !== hasLightBackgroundColor(theme)
) {
state.pydeckJson = JSON.parse(element.json)
state.pydeckJson = JSON5.parse(element.json)
state.id = element.id
}

Expand Down Expand Up @@ -204,7 +205,7 @@ export class DeckGlJsonChart extends PureComponent<PropsWithHeight, State> {
return false
}

const tooltip = JSON.parse(element.tooltip)
const tooltip = JSON5.parse(element.tooltip)

// NB: https://deckgl.readthedocs.io/en/latest/tooltip.html
if (tooltip.html) {
Expand Down

0 comments on commit 55ff72a

Please sign in to comment.