feat: enable `interval` selections for cartographic projections #6953

arvind · 2020-10-10T19:44:33Z

Chained to #7933.

📦 Published PR as canary version: 5.2.1--canary.6953.50ab21f.0

✨ Test out this PR locally via:

npm install vega-lite@5.2.1--canary.6953.50ab21f.0
# or 
yarn add vega-lite@5.2.1--canary.6953.50ab21f.0

dvmoritzschoefl · 2022-01-28T08:49:56Z

src/compile/selection/assemble.ts

+    const store: VgData = {name: selCmpt.name + STORE};
+
+    if (selCmpt.project.hasSelectionId) {
+      store.transform = [{type: 'collect', sort: {field: SELECTION_ID}}];


@arvind regarding #7932 and this PR I have a question:

Is the transform for datum.values[0] missing here? because you mentioned that the transform for the lasso must include this formula to correctly work with 'vlSelectionTuples'

"transform": [ {"type": "formula", "expr": "datum.values[0]", "as": "_vgsid_"}, {"type": "collect", "sort": {"field": "_vgsid_"}} ]

Yes, good eye! In #7933 and vega/vega#3397 (both of which this PR depends on), I introduced a more optimized format for selection stores when only the SELECTION_ID is being stored. In particular, after those two PRs, the selection store holds tuples in the form {_vgsid_, unit} sorted by _vgsid_. This structure allows us to skip the formula transform, as well all the array generation that would previously happen for the fields and values keys of the older tuples. The {unit, fields, values} tuple format still applied for non-selectionID based stores.

Hope that helps!

other models as geoshapes are likely to be background layer.

…ransparent fill to capture events

Workaround for vega/vega#3481

kanitw

Have some questions from a first skim. Will look more and add more comments.

kanitw · 2022-08-11T20:02:06Z

src/compile/selection/interval.ts

+      // Proxy scale reactions to ensure that an infinite loop doesn't occur
+      // when an interval selection filter touches the scale.
+      if (!scales.defined(selCmpt)) {
+        const triggerSg = name + SCALE_TRIGGER;


What is Sg short for? Please use full name if possible.

Short for signal. This is a convention used throughout src/compile/selection/ code. Do you want me to update this PR to fix it everywhere, send a separate PR, or keep it as is?

Keeping as is is fine but let's update to triggerSignal in a follow up pull request. sg could be misinterpreted as scenegraph.

kanitw · 2022-08-11T20:03:00Z

src/compile/selection/interval.ts

+        const triggerSg = name + SCALE_TRIGGER;
+        const scaleTriggers = channels.map(proj => {
+          const channel = proj.channel as 'x' | 'y';
+          const {data: dname, visual: vname} = proj.signals;


Why does visual become vname?

I wanted the variable name to clearly indicate that it was referencing the name of the visual signal.

kanitw · 2022-08-11T20:05:37Z

src/compile/selection/parse.ts

@@ -52,9 +52,10 @@ export function parseUnitSelection(model: UnitModel, selDefs: SelectionParameter
      events: isString(defaults.on) ? parseSelector(defaults.on, 'scope') : array(duplicate(defaults.on))
    } as any);

+    const def_ = duplicate(def); // defensive copy to prevent compilers from causing side effects


(Optional)

Do we need this because existing code caused side effects or because this PR introduces side effect?

If the latter, I'd prefer that we don't do this defensive copy, which seems alarming to me.

That said, if it's a lot of refactor to take this out, we can ignore this comment.

Yes, this PR introduces side effects — namely, if the interval selection parser detects a map projection, it rewrites the defaults from x/y projection to instead lat/lon and updates the predicate field to SELECTION_ID ready for subsequent selection parsers to operate. This was the cleanest, most scoped/localized way I could think of to introduce brushing on maps without triggering a more massive refactor since the rest of the interval selection components expect standard cartesian coordinate systems. So my preference would be to keep the defensive copy since its occurring just on the selection definitions.

kanitw · 2022-09-04T18:23:33Z

src/compile/unit.ts

@@ -281,7 +281,7 @@ export class UnitModel extends ModelWithField {
    if (this.encoding.x || this.encoding.y) {
      return 'cell';
    } else {
-      return undefined;
+      return 'view';


Any reasons why we're adding view here?

I'm trying to page this in since it's been a while but I believe it's to force a transparent background to be able to capture events. Without a 'view' style, there is no background specified on the overall view and so brushes only trigger if someone mouses down on a mark.

Yes, vega/vega#3480

domoritz · 2023-02-13T22:12:02Z

src/compile/selection/interval.ts

 import {UnitModel} from '../unit';
 import {assembleInit} from './assemble';
 import {SelectionProjection, TUPLE_FIELDS} from './project';
 import scales from './scales';

 export const BRUSH = '_brush';
 export const SCALE_TRIGGER = '_scale_trigger';
+export const GEO_INIT_TICK = 'geo_interval_init_tick'; // Workaround for https://github.com/vega/vega/issues/3481


Can you add something to the issue about how we need to update Vega-Lite when the issue is fixed in Vega?

domoritz

Looks good overall. Very exciting change!

domoritz · 2023-02-24T19:19:20Z

🎉

) Co-authored-by: GitHub Actions Bot <vega-actions-bot@users.noreply.github.com>

arvind mentioned this pull request Dec 15, 2020

add expr functions for extensional selection predicates vega/vega#3009

Merged

arvind mentioned this pull request Jan 7, 2021

Interval Selection for Projection #3587

Open

arvind force-pushed the as/geo-intervals branch 2 times, most recently from 36eafab to 9679a96 Compare February 8, 2021 00:02

arvind force-pushed the as/geo-intervals branch from 2e870be to 3a383f3 Compare January 19, 2022 18:24

This was referenced Jan 20, 2022

feat: optimize ID-driven selections #7933

Merged

Optimize selection functions for ID-only stores vega/vega#3397

Merged

arvind force-pushed the as/geo-intervals branch 3 times, most recently from dec30fa to 3002c88 Compare January 20, 2022 20:45

dvmoritzschoefl reviewed Jan 28, 2022

View reviewed changes

domoritz changed the base branch from master to next February 25, 2022 13:50

arvind force-pushed the as/geo-intervals branch from c385c46 to 0cc2660 Compare April 7, 2022 19:06

arvind added 2 commits April 28, 2022 10:17

Initial work to support geo-interval selections.

ec8176f

Preliminary geo intervals init.

7791c54

arvind force-pushed the as/geo-intervals branch from 923b0e3 to 7254d28 Compare April 28, 2022 18:58

arvind added 4 commits April 28, 2022 15:27

Update existing compile-time tests

f328e77

Mark geoshapes as interactive if there are selections present in

f036dae

other models as geoshapes are likely to be background layer.

Add compile-time tests for geo-interval signals

3005ad7

For non-cartesian groups, set a 'view' style (vega/vega#3480) for a t…

636dc88

…ransparent fill to capture events

arvind force-pushed the as/geo-intervals branch 4 times, most recently from 4f191c4 to 045358c Compare April 29, 2022 18:56

arvind and others added 5 commits June 13, 2022 16:50

Use projection center to init 1D geo intervals

bfea6fc

Initialize geo intervals with a timer signal

690b35e

Workaround for vega/vega#3481

Update compile-time tests

4a491f6

chore: update TOC [CI]

a4a12fe

chore: update examples [CI]

9f44f6d

arvind force-pushed the as/geo-intervals branch from d6cde6a to f5a2835 Compare June 13, 2022 20:56

arvind added 3 commits June 13, 2022 17:12

Add interactive geo brush examples

3db5481

fix: prevent selection compilers from side effecting selection defs

bda058e

fix: don't fire geo-brush intersection on unit signal updates

b837c81

arvind force-pushed the as/geo-intervals branch from 5bc6d6a to b837c81 Compare June 13, 2022 21:13

GitHub Actions Bot added 2 commits June 13, 2022 21:14

chore: update TOC [CI]

51fc546

chore: update examples [CI]

444301c

arvind marked this pull request as ready for review June 13, 2022 22:00

arvind requested a review from a team June 13, 2022 22:00

arvind force-pushed the as/geo-intervals branch from 3efcc9e to 331c1bd Compare June 16, 2022 19:44

Update runtime tests

50ab21f

arvind force-pushed the as/geo-intervals branch from 331c1bd to 50ab21f Compare June 16, 2022 19:49

kanitw reviewed Sep 4, 2022

View reviewed changes

domoritz changed the base branch from next to main February 13, 2023 22:06

domoritz reviewed Feb 13, 2023

View reviewed changes

domoritz approved these changes Feb 13, 2023

View reviewed changes

arvind merged commit e5cbe8f into main Feb 24, 2023

arvind deleted the as/geo-intervals branch February 24, 2023 17:53

binste mentioned this pull request May 26, 2023

Interactive charts - from a map! vega/altair#1232

Closed

BradyJ27 pushed a commit to BradyJ27/vega-lite that referenced this pull request Oct 19, 2023

feat: enable interval selections for cartographic projections (vega#6953

0a09ec3

) Co-authored-by: GitHub Actions Bot <vega-actions-bot@users.noreply.github.com>

arvind mentioned this pull request Oct 23, 2023

Support pan and zoom for geoshape mark #3306

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: enable `interval` selections for cartographic projections #6953

feat: enable `interval` selections for cartographic projections #6953

arvind commented Oct 10, 2020 •

edited by github-actions bot

Loading

dvmoritzschoefl Jan 28, 2022

arvind Jan 28, 2022

kanitw left a comment

kanitw Aug 11, 2022

arvind Jan 5, 2023

domoritz Feb 13, 2023

kanitw Aug 11, 2022

arvind Jan 5, 2023

kanitw Aug 11, 2022

arvind Jan 5, 2023

kanitw Sep 4, 2022

arvind Jan 5, 2023

arvind Jan 24, 2023

domoritz Feb 13, 2023

arvind Feb 24, 2023

domoritz left a comment

domoritz commented Feb 24, 2023

feat: enable interval selections for cartographic projections #6953

feat: enable interval selections for cartographic projections #6953

Conversation

arvind commented Oct 10, 2020 • edited by github-actions bot Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kanitw left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

domoritz left a comment

Choose a reason for hiding this comment

domoritz commented Feb 24, 2023

feat: enable `interval` selections for cartographic projections #6953

feat: enable `interval` selections for cartographic projections #6953

arvind commented Oct 10, 2020 •

edited by github-actions bot

Loading