Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add hint when add rows is used on vega lite chart #8499

Closed
wants to merge 1 commit into from
Closed

Add hint when add rows is used on vega lite chart #8499

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions frontend/lib/src/AppNode.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,7 @@ export class ElementNode implements AppNode {
datasets: modifiedDatasets,
useContainerWidth: proto.useContainerWidth,
vegaLiteTheme: proto.theme,
needsAddRows: proto.needsAddRows,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is the proto field here even required? Couldn't we just always set it to false here since the proto change seems to be never set to something else than false?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yea, the value needs to be set. Practically, this will always be false, but I figured it's best to maintain the protobuf's members (theoretically, people can create a custom message, and we can honor it). See example.

}

this.lazyVegaLiteChartElement = toReturn
Expand Down Expand Up @@ -319,6 +320,8 @@ export class ElementNode implements AppNode {
? draft.data.addRows(newDataSetQuiver)
: newDataSetQuiver
}

draft.needsAddRows = true
})
}
}
Expand Down
22 changes: 16 additions & 6 deletions frontend/lib/src/components/elements/ArrowVegaLiteChart/ArrowVegaLiteChart.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,8 @@ export interface VegaLiteChartElement {

/** override the properties with a theme. Currently, only "streamlit" or None are accepted. */
vegaLiteTheme: string

needsAddRows: boolean
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: can you also add a comment here describing the property?

}

/** A mapping of `ArrowNamedDataSet.proto`. */
Expand Down Expand Up @@ -186,9 +188,10 @@ export class ArrowVegaLiteChart extends PureComponent<PropsWithHeight, State> {

const prevData = prevElement.data
const { data } = element
const addRowsHint = element.needsAddRows ?? false

if (prevData || data) {
this.updateData(this.defaultDataName, prevData, data)
this.updateData(this.defaultDataName, prevData, data, addRowsHint)
}

const prevDataSets = getDataSets(prevElement) || {}
Expand All @@ -198,13 +201,13 @@ export class ArrowVegaLiteChart extends PureComponent<PropsWithHeight, State> {
const datasetName = name || this.defaultDataName
const prevDataset = prevDataSets[datasetName]

this.updateData(datasetName, prevDataset, dataset)
this.updateData(datasetName, prevDataset, dataset, addRowsHint)
}

// Remove all datasets that are in the previous but not the current datasets.
for (const name of Object.keys(prevDataSets)) {
if (!dataSets.hasOwnProperty(name) && name !== this.defaultDataName) {
this.updateData(name, null, null)
this.updateData(name, null, null, addRowsHint)
}
}

Expand Down Expand Up @@ -260,7 +263,8 @@ export class ArrowVegaLiteChart extends PureComponent<PropsWithHeight, State> {
private updateData(
name: string,
prevData: Quiver | null,
data: Quiver | null
data: Quiver | null,
addRowsHint: boolean
): void {
if (!this.vegaView) {
throw new Error("Chart has not been drawn yet")
Expand Down Expand Up @@ -293,7 +297,8 @@ export class ArrowVegaLiteChart extends PureComponent<PropsWithHeight, State> {
prevNumCols,
data,
numRows,
numCols
numCols,
addRowsHint
)
) {
if (prevNumRows < numRows) {
Expand Down Expand Up @@ -502,7 +507,8 @@ function dataIsAnAppendOfPrev(
prevNumCols: number,
data: Quiver,
numRows: number,
numCols: number
numCols: number,
addRowsHint: boolean
): boolean {
// Check whether dataframes have the same shape.

Expand All @@ -525,6 +531,10 @@ function dataIsAnAppendOfPrev(
const c = numCols - 1
const r = prevNumRows - 1

if (!addRowsHint) {
return false
}

// Check if the new dataframe looks like it's a superset of the old one.
// (this is a very light check, and not guaranteed to be right!)
if (
Expand Down
2 changes: 2 additions & 0 deletions lib/streamlit/elements/arrow_vega_lite.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,8 @@ def marshall(
if data is not None:
arrow.marshall(proto.data, data)

proto.needs_add_rows = False


# See https://vega.github.io/vega-lite/docs/encoding.html
_CHANNELS = {
Expand Down
2 changes: 2 additions & 0 deletions proto/streamlit/proto/ArrowVegaLiteChart.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,5 +40,7 @@ message ArrowVegaLiteChart {
// override the properties with a theme. Currently, only "streamlit" or None are accepted.
string theme = 6;

bool needs_add_rows = 7;
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: can you add a comment here describing the property?


reserved 3;
}
Loading