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

box plot tips #1839

Closed
wants to merge 3 commits into from
Closed

box plot tips #1839

wants to merge 3 commits into from

Conversation

Fil
Copy link
Contributor

@Fil Fil commented Aug 27, 2023

For each box plot we have 5 elements to report (+ the outliers).

When there are less than 5 values, a choice must be made. The "report" array is in order of priority (highest priority last, since we use Array.pop).

This seems to work well with 1, 2, 3, 4, and 5 values. I wonder if q1 and q3 make sense when there are < 5 values, by the way, and we should probably censor them in the visible mark too?

closes #1835

(also adds a test for boxY)

we have 5 elements to report + the outliers

when there are less than 5 values, a choice must be made. The "report" array is in order of priority (highest priority last, since we use Array.pop).

This seems to work well with 1, 2, 3, 4, and 5 values. I wonder if q1 and q3 make sense when there are < 5 values, by the way, and we should probably censor them in the visible mark too?
@Fil Fil requested review from mbostock and tophtucker August 27, 2023 13:26
Copy link
Contributor

@tophtucker tophtucker left a comment

Choose a reason for hiding this comment

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

Tip behavior works exactly like I’d expect. 👍

boxStats recomputes some things that the other marks were already computing; I guess it’d be more efficient to precompute them all and then pass them into both the graphical marks and the tip. But, as ever, I’m not sure how much it matters. 😄

Copy link
Member

@mbostock mbostock left a comment

Choose a reason for hiding this comment

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

I would like to take a look but haven’t had a chance yet.

Copy link
Member

@mbostock mbostock left a comment

Choose a reason for hiding this comment

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

I tried this on a non-faceted plot:

Plot.plot({
  y: {
    interval: 0.1
  },
  marks: [
    Plot.boxX(
      olympians.filter((d) => d.height),
      {x: "weight", y: "height", tip: true}
    )
  ]
})

I don’t think it’s functioning as intended in this case? I was able to hover points within the box:

Screen.Recording.2023-08-29.at.3.49.24.PM.mov

It also feels to me like the pointer modes are switched. For boxX, I think we want pointerY so that you can brush horizontally and it sticks to the current box; likewise we want pointerX for boxY. Unfortunately when using faceting instead of position, it looks like pointerX and pointerY doesn’t really make a difference (since #1777); I wonder if we want to visit that so y and fy behave the same way? 🤔

Design-wise, I think I would prefer one combined tip per box with the summary statistics labeled (min, q1, median, q3, max). And maybe not bother giving a tip to the outliers? You could always add an invisible dot mark overlay if you wanted to label all the points.

@Fil
Copy link
Contributor Author

Fil commented Aug 30, 2023

the pointer modes are switched

Very true (and fixed).

EDIT The remainder of this comment is now a stand-alone issue #1842

For the test case with interval on the y channel, I think the problem is with the map transform, which groups by y and not by intervaled y. If you replace `y: "height"` by `y: (d) => ((d.height * 10) | 0) / 10` it works. But that means the box mark is broken, too — we didn't see it because it just returns more outliers than necessary.

Here's a test with the correct outliers:

export async function boxplotX() {
  const olympians = await d3.csv<any>("data/athletes.csv", d3.autoType);
  return Plot.plot({
    y: {interval: 0.1},
    marks: [
      Plot.boxX(
        olympians.filter((d) => d.height),
        {x: "weight", y: "height", z: (d) => Math.floor(d.height * 10)}
      )
    ]
  });
}

correct

now, remove the z specified above, and you get this incorrect chart, where you have "outliers" inside the q1-q3 band:

incorrect

@mbostock
Copy link
Member

Sounds like the map transform needs to use maybeApplyInterval, like the group transform does? Can you file a bug? 🙏

@Fil Fil marked this pull request as draft August 31, 2023 09:13
@Fil
Copy link
Contributor Author

Fil commented Aug 31, 2023

An alternative with a summary table: #1843

@Fil
Copy link
Contributor Author

Fil commented Sep 13, 2023

closing in favor of #1843

@Fil Fil closed this Sep 13, 2023
@Fil Fil deleted the fil/fix-box-tip branch September 13, 2023 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tips are chaotic on compound marks
3 participants