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.

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

Scatterplot fixes and improvements #11898

Merged
merged 13 commits into from
Jan 3, 2025
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { Pattern } from '@/util/ast/match'
import { getTextWidthBySizeAndFamily } from '@/util/measurement'
import { defineKeybinds } from '@/util/visualizationBuiltins'
import { computed, ref, watch, watchEffect, watchPostEffect } from 'vue'
import { ToolbarItem } from './toolbar'

export const name = 'Scatter Plot'
export const icon = 'points'
Expand Down Expand Up @@ -707,12 +708,12 @@ watchPostEffect(() => {
const yScale_ = yScale.value
const plotData = getPlotData(data.value) as Point[]
const series = Object.keys(data.value.axis).filter((s) => s != 'x')
const colorScale = (d: string) => {
const colorScale = (d: Point) => {
const color = d3.scaleOrdinal(d3.schemeCategory10).domain(series)
if (data.value.is_multi_series) {
return color(d)
return color(d.series ?? '')
}
return DEFAULT_FILL_COLOR
return d.color ?? DEFAULT_FILL_COLOR
}
d3Points.value
.selectAll<SVGPathElement, unknown>('path')
Expand All @@ -730,7 +731,7 @@ watchPostEffect(() => {
'd',
symbol.type(matchShape).size((d) => (d.size ?? 0.15) * SIZE_SCALE_MULTIPLER),
)
.style('fill', (d) => colorScale(d.series || ''))
.style('fill', (d) => colorScale(d))
.attr('transform', (d) => `translate(${xScale_(Number(d.x))}, ${yScale_(d.y)})`)
if (data.value.points.labels === VISIBLE_POINTS) {
d3Points.value
Expand Down Expand Up @@ -865,42 +866,49 @@ const makeSeriesLabelOptions = () => {
return seriesOptions
}

config.setToolbar([
{
icon: 'select',
title: 'Enable Selection',
toggle: selectionEnabled,
},
{
icon: 'show_all',
title: 'Fit All',
onClick: () => zoomToSelected(false),
},
{
icon: 'zoom',
title: 'Zoom to Selected',
disabled: () => brushExtent.value == null,
onClick: zoomToSelected,
},
{
icon: 'add_to_graph_editor',
title: 'Create component of selected points',
disabled: () => !createNewFilterNodeEnabled.value,
onClick: createNewFilterNode,
},
{
type: 'textSelectionMenu',
selectedTextOption: yAxisSelected,
title: 'Choose Y Axis Label',
heading: 'Y Axis Label: ',
options: {
none: {
label: 'No Label',
},
...makeSeriesLabelOptions(),
const createTextSelectionButton = (): ToolbarItem => ({
type: 'textSelectionMenu',
selectedTextOption: yAxisSelected,
title: 'Choose Y Axis Label',
heading: 'Y Axis Label: ',
options: {
none: {
label: 'No Label',
},
...makeSeriesLabelOptions(),
},
])
})

function useScatterplotVizToolbar() {
const textSelectionButton = createTextSelectionButton()
return computed(() => [
{
icon: 'select',
title: 'Enable Selection',
toggle: selectionEnabled,
},
{
icon: 'show_all',
title: 'Fit All',
onClick: () => zoomToSelected(false),
},
{
icon: 'zoom',
title: 'Zoom to Selected',
disabled: () => brushExtent.value == null,
onClick: zoomToSelected,
},
{
icon: 'add_to_graph_editor',
title: 'Create component of selected points',
disabled: () => !createNewFilterNodeEnabled.value,
onClick: createNewFilterNode,
},
...(data.value.is_multi_series ? [textSelectionButton] : []),
])
}

config.setToolbar(useScatterplotVizToolbar())
</script>

<template>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,10 @@ type Point_Data
candidates.find if_missing=candidates.first is_good
Point_Data.Row_Number ->
Point_Data.iota table.row_count
Point_Data.Color ->
colour_col = table.lookup_ignore_case "Colour"
is_good c = (self.is_recognized c).not
if is_good colour_col then colour_col else Error.throw Nothing
Copy link
Member

Choose a reason for hiding this comment

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

Looks much better.

Btw. not directly related to this change but I see that in this method, if the column is not found, we sometimes throw Nothing and sometimes throw No_Fallback_Column. Signature suggests No_Fallback_Column, so if possible would be good to get this consistent (although tbh it's not that important as long as it doesn't break anything since it's just internal code - still would be good to make it clearer if possible).


_ -> Error.throw No_Fallback_Column

Expand Down
15 changes: 15 additions & 0 deletions test/Visualization_Tests/src/Scatter_Plot_Spec.enso
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,21 @@ add_specs suite_builder =
table = Table.from_rows header [row_1, row_2]
expect table (labels index 'y') '[{"size":40,"x":0,"y":10, "row_number": 0},{"size":50,"x":1,"y":20, "row_number": 1}]' '"get_row"'

group_builder.specify "recognises color and colour for the colour column, colour" <|
header = [ 'u' , 'k' , 'size', 'colour']
row_1 = [ 10 , 3 , 40, 'black']
row_2 = [ 20 , 4 , 50, 'white']
table = Table.from_rows header [row_1, row_2]
expect table (labels 'u' 'k') '[{"x":10,"y":3,"color":"black","size":40,"row_number":0},{"x":20,"y":4,"color":"white","size":50,"row_number":1}]' '"get_row"'

group_builder.specify "recognizes color and colour for the colour column, color" <|
header = [ 'u' , 's' , 'size', 'color']
row_1 = [ 10 , 3 , 40, 'red']
row_2 = [ 20 , 4 , 50, 'white']
row_3 = [ 30 , 5 , 50, 'blue']
table = Table.from_rows header [row_1, row_2, row_3]
expect table (labels 'u' 's') '[{"x":10,"y":3,"color":"red", "size":40,"row_number": 0},{"x":20,"y":4,"color":"white", "size":50,"row_number": 1}, {"x":30,"y":5,"color":"blue", "size":50,"row_number": 2}]' '"get_row"'

group_builder.specify "using indices for x if given a vector" <|
vector = [0,10,20]
expect vector no_labels '[{"x":0,"y":0, "row_number": 0},{"x":1,"y":10, "row_number": 1},{"x":2,"y":20, "row_number": 2}]' '"at"'
Expand Down
Loading