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

Don’t ignore undefined values for ordinal dimensions, and make grouping and faceting consistent? #45

Closed
mbostock opened this issue Dec 11, 2020 · 3 comments
Labels
bug Something isn’t working

Comments

@mbostock
Copy link
Member

Marks currently ignore data with undefined values. For example, BarX will ignore data whose y, x1 or x2 value is undefined.

const index = filter(I, ...this._positions(channels), F, S);

This is a handy feature for avoiding SVG errors. However, we’re not entirely inconsistent with grouping. I fixed the group1 transform to skip groups whose key is undefined,

return data => groups(data, key).filter(defined1);

but I forgot to fix group2 at the same time. And we don’t apply this same technique to faceting, which is also inconsistent.

I’m now of the opinion that ignoring undefined data for ordinal dimensions — which includes grouping and faceting — is undesirable. Whereas there’s no consistent way to handle undefined data in a quantitative dimension, undefined values don’t have to be specially treated in ordinal dimensions. Furthermore, null and undefined are often used to represent an “other” or “unknown” category with grouping and faceting.

Therefore, I think we should changing the filtering logic and only filter undefined values for quantitative dimensions.

@Fil
Copy link
Contributor

Fil commented Dec 16, 2020

related #52

@mbostock mbostock added the bug Something isn’t working label Feb 24, 2021
@mbostock mbostock added this to the Friends Preview milestone Feb 24, 2021
@Fil Fil self-assigned this Mar 3, 2021
@mbostock
Copy link
Member Author

mbostock commented Mar 8, 2021

This needs to be fixed at the same time as #52. If we allow the group transform to return groups with null or undefined keys, the bar mark will currently refuse to display them:

Screen Shot 2021-03-08 at 2 10 19 PM

Whereas if the bar mark tests the scaled key to determine if the position is defined, then it will work as desired:

Screen Shot 2021-03-08 at 2 10 12 PM

Plot.plot({
    y: {
      grid: true
    },
    marks: [
      Plot.groupBarY(data, {x: "sex"}),
      Plot.ruleY([0])
    ]
  })

If the null values are not desired, then the x-domain can be specified explicitly to not include null; however, the x-domain should also be made available as an option to the group transform so these values can be ignored. Otherwise the y-scale may have the wrong inferred domain. I’ll file a separate issue for a domain option for the group transform.

@mbostock mbostock removed this from the Friends Preview milestone Mar 10, 2021
Fil added a commit that referenced this issue Mar 24, 2021
…llowing NaN, null and undefined as (ordinal) classes

fixes #52
fixes #45
Fil added a commit that referenced this issue Mar 24, 2021
…llowing NaN, null and undefined as (ordinal) classes

fixes #52
fixes #45
@Fil Fil mentioned this issue Mar 24, 2021
@Fil Fil removed their assignment Mar 26, 2021
Fil added a commit that referenced this issue Mar 27, 2021
…llowing NaN, null and undefined as (ordinal) classes

groups respect the domain option

fixes #52
fixes #45
fixes #255
supersedes #272
Fil added a commit that referenced this issue Mar 27, 2021
…llowing NaN, null and undefined as (ordinal) classes

groups respect the domain option

fixes #52
fixes #45
fixes #255
supersedes #271
@mbostock
Copy link
Member Author

As of #272, we’re generating the group for the null/undefined keys, but the bar mark is refusing to display them. So this issue is fixed, but we still need to work on #52.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn’t working
Projects
None yet
2 participants