feat: more options for how marks and scales represent invalid data (e.g., nulls, NaNs) #9342

kanitw · 2024-05-07T20:57:38Z

Add more spec options for handling invalid data
- extend mark.invalid
- add new config.scale.invalid
See docs's pdf file from invalid.md or the actual doc for explanation of the behavior. (Given the complexity of the PR, reviewers should check out the branch and test the spec locally.)

refactor:

Adjust how we generate rules for invalid data to handle these options, per channel type.
- data's invalid filter
- defined encoding (for breaking paths)
Make wrapCondition accept invalidValueRef for "include" mode.
Remove the hidden/screen "hide" mode for invalid data (Given this is a hidden feature, it's not a breaking change.)## PR Description

bug fixes:

fix mark.invalid set the invalid points on y channel at the top #8254 -- correctly place invalid point as zero, when included.

Known issues / Follow up items:

Tooltip doesn't show null correctly (Format of null data in tooltips. #6417) -- I'll fix this separately in a follow up PR
Should Vega-lite always include color for invalid by default? (Should Vega-lite always include color for invalid by default? #9351)

Checklist

Tests:

Integration tests -- added a bunch of examples
Unit tests (in progress) -- I'll re-review the code and add more unit tests.

Docs:

Has documentation under site/docs/ + examples.

cloudflare-pages · 2024-05-07T20:57:47Z

Deploying vega-lite with Cloudflare Pages

Latest commit:	`1511d15`
Status:	✅ Deploy successful!
Preview URL:	https://ac50a1aa.vega-lite.pages.dev
Branch Preview URL:	https://kw-9325-better-mark-invalid.vega-lite.pages.dev

View logs

kanitw · 2024-05-07T22:03:18Z

src/compile/data/filterinvalid.ts

-          // While discrete domain scales can handle invalid values, continuous scales can't.
-          // Thus, for non-path marks, we have to filter null for scales with continuous domains.
-          // (For path marks, we will use "defined" property and skip these values instead.)
-          if (hasContinuousDomain(scaleType) && fieldDef.aggregate !== 'count' && !isPathMark(mark)) {


These rules are now part of getScaleInvalidDataMode

kanitw · 2024-05-07T22:06:32Z

src/compile/data/parse.ts

@@ -384,13 +399,41 @@ export function parseData(model: Model): DataComponent {
    head = StackNode.makeFromEncoding(head, model) ?? head;
  }

+  let preFilterInvalid: OutputNode | undefined;


TODO: file a separate ticket

Separate from this PR, I have a hunch that this will remain slightly incorrect for stacked chart,
since null values will be "stacked", but it's probably not worth changing the behavior in this PR.

kanitw · 2024-05-07T22:10:20Z

src/compile/data/parse.ts

FYI, in this file, we're creating data sources pre-/post-filter invalid values, depending on the invalid data mode.

kanitw · 2024-05-07T22:11:18Z

src/compile/guide.ts

    return {
      ...encode,
-      ...wrapCondition(model, valueDef, channel, def => signalOrValueRef(def.value))
+      ...wrapCondition({


FYI, no behavior change here. I just switch wrapCondition to use parameter argument and make it explicit that guideEncodeEntry won't need the new invalidValueRef added.

kanitw · 2024-05-07T22:11:34Z

src/compile/mark/encode/aria.ts

@@ -45,7 +45,13 @@ export function description(model: UnitModel) {
  const channelDef = encoding.description;

  if (channelDef) {
-    return wrapCondition(model, channelDef, 'description', cDef => textRef(cDef, model.config));
+    return wrapCondition({


FYI, no behavior change here. I just switch wrapCondition to use parameter argument and make it explicit that aria won't need the new invalidValueRef added.

kanitw · 2024-05-07T22:19:56Z

src/compile/mark/encode/defined.ts

      const field = model.vgField(channel, {expr: 'datum', binSuffix: model.stack?.impute ? 'mid' : undefined});
-
-      // While discrete domain scales can handle invalid values, continuous scales can't.
-      if (field && hasContinuousDomain(scaleType)) {


All the continuous scale check is now in getScaleInvalidDataMode

kanitw · 2024-05-07T22:24:24Z

src/compile/mark/encode/nonposition.ts


-  return wrapCondition(model, channelDef, vgChannel ?? channel, cDef => {
+  const invalidValueRef = getConditionalValueRefForIncludingInvalidValue({


FYI, the main change in this file is adding invalidValueRef here.

kanitw · 2024-05-07T22:36:59Z

src/compile/scale/domain.ts

@@ -249,6 +251,11 @@ function parseSingleChannelDomain(
  const {type} = fieldOrDatumDef;
  const timeUnit = fieldOrDatumDef['timeUnit'];

+  const dataSourceTypeForScaleDomain = getScaleDataSourceForHandlingInvalidValues({


FYI, this is the main change in this file. Instead of always drawing scale domains from "main" data source, we may choose pre-/post- filtering invalid values, depending on the invalid data mode.

kanitw · 2024-05-07T22:37:31Z

src/mark.ts

-/** @hidden */
-export type Hide = 'hide';
-
-export interface MarkInvalidMixins {


FYI, moved to src/invalid.ts + revamped

kanitw · 2024-05-07T22:39:17Z

test/compile/mark/text.test.ts

-          field: 'mean_Acceleration'
-        }
-      ]);
+      expect(props.fill).toEqual({


Intended change -- we no longer relying "null" fill to hide invalid marks in the new "break-paths-keep-domains" that replaces in the old secret "hide" mode.

kanitw · 2024-05-07T22:40:22Z

src/mark.ts

@@ -72,18 +77,6 @@ export interface TooltipContent {
  content: 'encoding' | 'data';
 }

-/** @hidden */
-export type Hide = 'hide';


FYI, the old hide mode was a secret experimental feature (not part of public API) that I experimented with many years ago. It's not documented and thus can be removed.

kanitw · 2024-05-07T22:40:41Z

test/compile/selection/layers.test.ts

-                field: 'Origin'
-              }
-            ],
+            fill: {


FYI, intended change -- we no longer relying "null" fill to hide invalid marks in the new "break-paths-keep-domains" that replaces in the old secret "hide" mode.

Same for the rest of this file.

kanitw · 2024-05-07T22:50:42Z

src/compile/mark/encode/conditional.ts

+  model,
+  channelDef,
+  vgChannel,
+  invalidValueRef,


FYI, the main change in this file is to add invalidValueRef that caller of this method may pass in.

However, I also switch to object argument so it's easy to make explicit when invalidValueRef is not used.

kanitw · 2024-05-07T22:51:12Z

src/compile/invalid/datasources.ts

+  isPath: boolean;
+}
+
+export function getDataSourcesForHandlingInvalidValues({


This method includes one of the main rules for defining how these modes affect data sources for scale / marks.

kanitw · 2024-05-07T22:51:44Z

src/compile/mark/encode/base.ts

@@ -45,27 +39,7 @@ export function baseEncodeEntry(model: UnitModel, ignore: Ignore) {
  };
 }

-// TODO: mark VgValueRef[] as readonly after https://github.com/vega/vega/pull/1987
-function wrapAllFieldsInvalid(model: UnitModel, channel: Channel, valueRef: VgValueRef | VgValueRef[]): VgEncodeEntry {


FYI, all old code in this file were for the legacy secret/hidden "hide" mode that I remove.

The old hide mode hides data by adjusting fill/stroke, which still wastes rendering time to draw transparent marks.
With the new break-paths-keep-domain, we're switching to rely on filters to produce this behavior.

kanitw · 2024-05-09T05:23:26Z

examples/compiled/bar_distinct.vg.json

@@ -16,10 +16,6 @@
          "ops": ["distinct"],
          "fields": ["Name"],
          "as": ["distinct_Name"]
-        },
-        {


FYI, correct change since distinct aggregate can't output nulls.

kanitw · 2024-05-09T19:51:59Z

examples/compiled/bar_grouped_repeated.vg.json

@@ -52,7 +52,8 @@
          "y": [
            {
              "test": "!isValid(datum[\"sum_Worldwide Gross\"]) || !isFinite(+datum[\"sum_Worldwide Gross\"])",
-              "field": {"group": "height"}
+              "scale": "y",


FYI, Good change as intended in a later fix commit -- invalid values get placed as zero rather than min. Same for many other diffs.

kanitw · 2024-05-09T19:53:13Z

examples/compiled/point_invalid_color.vg.json

@@ -32,14 +32,16 @@
          "x": [
            {
              "test": "!isValid(datum[\"IMDB Rating\"]) || !isFinite(+datum[\"IMDB Rating\"])",
+              "scale": "x",


FYI, good change as intended in a later fix commit -- invalid values get placed as zero rather than min.

value: 0 = min

scale: 'x', value: '0' -> encode same output as 0

kanitw · 2024-05-09T20:50:15Z

examples/compiled/test_invalid_default.png

FYI, I adjust the example layout a bit, so they fit better in the doc page.

Also adjusted data.

kanitw · 2024-05-09T20:59:21Z

site/static/main.css

Added for showing code only in the new doc page added.

kanitw · 2024-05-09T21:03:37Z

test/compile/selection/parse.test.ts

No actual change here, but just change the assertion to be easier to debug as my older code fails the assertion in this file.

…nvalid # Conflicts: # build/vega-lite-schema.json # src/scale.ts

lsh · 2024-05-17T22:08:06Z

src/compile/scale/component.ts

-  public domainDefinitelyIncludesZero() {
-    if (this.get('zero') !== false) {
-      return true;
+  public domainHasZero(): 'definitely' | 'definitely-not' | 'maybe' {


nit: I'm not super sure about the wording here in this union, but non blocking either way.

yeah, i like it better than boolean | null where null is maybe though. Given it's internal, we can always change later.

lsh · 2024-05-17T22:11:05Z

src/invalid.ts

+  | 'filter'
+  | 'break-paths-filter-domains'
+  | 'break-paths-show-domains'
+  | 'break-paths-and-show-path-domains'


Why is this the only one that includes the word and? Several of these options feel like conjunctive payloads, but this one is special for some reason?

good call! fixed.

lsh

Ham and I talked about the design here for a while yesterday and I'm convinced it's extensible and composable. Since he addressed my main nits about syntax, I'm good with these changes.

…to avoid unreachable lines

kanitw force-pushed the kw/9325-better-mark-invalid branch from 6198c53 to e28256c Compare May 7, 2024 22:02

kanitw commented May 7, 2024

View reviewed changes

kanitw force-pushed the kw/9325-better-mark-invalid branch from e28256c to 9f1bc85 Compare May 7, 2024 22:49

kanitw commented May 7, 2024

View reviewed changes

kanitw force-pushed the kw/9325-better-mark-invalid branch 10 times, most recently from 264399f to 414e175 Compare May 9, 2024 04:59

kanitw commented May 9, 2024

View reviewed changes

kanitw mentioned this pull request May 9, 2024

mark.invalid set the invalid points on y channel at the top #8254

Closed

kanitw force-pushed the kw/9325-better-mark-invalid branch from 3c41e6c to bd9378a Compare May 9, 2024 15:08

kanitw changed the title ~~feat: more options for handling invalid~~ feat: more options for handling invalid data May 9, 2024

kanitw commented May 9, 2024

View reviewed changes

kanitw force-pushed the kw/9325-better-mark-invalid branch 4 times, most recently from c038fd2 to e0d2a64 Compare May 9, 2024 22:45

kanitw commented May 9, 2024

View reviewed changes

kanitw force-pushed the kw/9325-better-mark-invalid branch 2 times, most recently from 2407cdd to e25ba16 Compare May 10, 2024 16:00

fix: add missing .vl to a filename

bb8ff99

kanitw force-pushed the kw/9325-better-mark-invalid branch from df34cdf to bb8ff99 Compare May 16, 2024 02:40

chore: update examples [CI]

42b2175

kanitw mentioned this pull request May 16, 2024

Should Vega-lite always include color for invalid by default? #9351

Open

kanitw and others added 2 commits May 17, 2024 15:02

Merge remote-tracking branch 'origin/main' into kw/9325-better-mark-i…

df46191

…nvalid # Conflicts: # build/vega-lite-schema.json # src/scale.ts

chore: update examples [CI]

fd8286d

lsh reviewed May 17, 2024

View reviewed changes

kanitw and others added 3 commits May 17, 2024 15:43

fix: remove "and" from 'break-paths-show-path-domains'

bea7425

chore: update schema [CI]

540951d

chore: update examples [CI]

744c346

kanitw changed the title ~~feat: more options for handling invalid data~~ feat: more options for how marks and corresponding scales represent invalid data May 18, 2024

docs: add more examples for other relevant techniques

8dcaeae

kanitw changed the title ~~feat: more options for how marks and corresponding scales represent invalid data~~ feat: more options for how marks and scales represent invalid data (e.g., nulls, NaNs) May 18, 2024

GitHub Actions Bot added 4 commits May 18, 2024 17:39

chore: update TOC [CI]

8646d1a

chore: update examples [CI]

362f694

style: auto-formatting [CI]

4cbce89

chore: update TOC [CI]

5de9c72

lsh reviewed May 18, 2024

View reviewed changes

lsh approved these changes May 18, 2024

View reviewed changes

kanitw and others added 7 commits May 18, 2024 14:20

fix: rule re: whether the domain has zero or not + how to axis scale min

7bc0ee2

chore: update examples [CI]

3c1b26c

docs: fix docs

6ab0f7c

style: auto-formatting [CI]

0d73d4b

refactor: split scaledZeroOrMinOrMax from zeroOrMinOrMaxPosition …

1511d15

…to avoid unreachable lines

add more examples

a2c3418

chore: update examples [CI]

14696bb

kanitw enabled auto-merge (squash) May 20, 2024 04:23

kanitw merged commit 502db1d into main May 20, 2024
8 checks passed

kanitw deleted the kw/9325-better-mark-invalid branch May 20, 2024 04:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: more options for how marks and scales represent invalid data (e.g., nulls, NaNs) #9342

feat: more options for how marks and scales represent invalid data (e.g., nulls, NaNs) #9342

kanitw commented May 7, 2024 •

edited

cloudflare-pages bot commented May 7, 2024 •

edited

kanitw May 7, 2024

kanitw May 7, 2024 •

edited

kanitw May 7, 2024

kanitw May 7, 2024

kanitw May 7, 2024

kanitw May 7, 2024

kanitw May 7, 2024

kanitw May 7, 2024

kanitw May 7, 2024

kanitw May 7, 2024

kanitw May 7, 2024

kanitw May 7, 2024

kanitw May 7, 2024

kanitw May 7, 2024

kanitw May 7, 2024

kanitw May 9, 2024 •

edited

kanitw May 9, 2024 •

edited

kanitw May 9, 2024

kanitw May 9, 2024

kanitw May 9, 2024

kanitw May 9, 2024

lsh May 17, 2024

kanitw May 17, 2024

lsh May 17, 2024

kanitw May 17, 2024

lsh left a comment


		return wrapCondition(model, channelDef, vgChannel ?? channel, cDef => {
		const invalidValueRef = getConditionalValueRefForIncludingInvalidValue({

feat: more options for how marks and scales represent invalid data (e.g., nulls, NaNs) #9342

feat: more options for how marks and scales represent invalid data (e.g., nulls, NaNs) #9342

Conversation

kanitw commented May 7, 2024 • edited

Checklist

cloudflare-pages bot commented May 7, 2024 • edited

Deploying vega-lite with Cloudflare Pages

Choose a reason for hiding this comment

kanitw May 7, 2024 • edited

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

Choose a reason for hiding this comment

kanitw May 9, 2024 • edited

Choose a reason for hiding this comment

kanitw May 9, 2024 • edited

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

lsh left a comment

Choose a reason for hiding this comment

kanitw commented May 7, 2024 •

edited

cloudflare-pages bot commented May 7, 2024 •

edited

kanitw May 7, 2024 •

edited

kanitw May 9, 2024 •

edited

kanitw May 9, 2024 •

edited