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

docs: document .funnel/funnel argument #513

Merged
merged 2 commits into from
Jan 28, 2025
Merged

docs: document .funnel/funnel argument #513

merged 2 commits into from
Jan 28, 2025

Conversation

maelle
Copy link
Collaborator

@maelle maelle commented Jan 28, 2025

No description provided.

@maelle maelle requested a review from krlmlr January 28, 2025 09:10
@maelle maelle changed the title docs: document .funnel/`funnel argument docs: document .funnel/funnel argument Jan 28, 2025
Copy link
Contributor

This is how benchmark results would change (along with a 95% confidence interval in relative change) if a29e651 is merged into main:

  • ✔️001_tpch_01: 21.8ms -> 21.4ms [-6.77%, +2.95%]
  • ✔️001_tpch_02: 61.6ms -> 61.6ms [-0.64%, +0.79%]
  • ✔️001_tpch_03: 36.8ms -> 36.9ms [-0.64%, +0.9%]
  • ✔️001_tpch_04: 21.1ms -> 21ms [-2.38%, +1.86%]
  • ✔️001_tpch_05: 54.9ms -> 54.9ms [-0.98%, +0.76%]
  • ✔️001_tpch_06: 17.1ms -> 17ms [-4.1%, +2.96%]
  • ✔️001_tpch_07: 69.1ms -> 69ms [-1.03%, +0.77%]
  • ✔️001_tpch_08: 88.7ms -> 88.4ms [-1.47%, +0.86%]
  • ✔️001_tpch_09: 71ms -> 71.1ms [-0.96%, +1.06%]
  • ✔️001_tpch_10: 46.4ms -> 46.4ms [-0.84%, +0.89%]
  • ✔️001_tpch_11: 30ms -> 30ms [-1.02%, +0.85%]
  • ✔️001_tpch_12: 20.6ms -> 21ms [-0.76%, +5.01%]
  • ✔️001_tpch_13: 23.5ms -> 24ms [-0.65%, +5.17%]
  • ✔️001_tpch_14: 22.5ms -> 22.5ms [-2.53%, +3.07%]
  • ✔️001_tpch_15: 28.5ms -> 28.2ms [-2.28%, +0.23%]
  • ✔️001_tpch_16: 27.8ms -> 27.9ms [-1.17%, +1.78%]
  • ✔️001_tpch_17: 24ms -> 23.7ms [-2.06%, +0.28%]
  • ✔️001_tpch_18: 21.6ms -> 21.9ms [-0.59%, +3.11%]
  • ✔️001_tpch_19: 43.2ms -> 43ms [-1.22%, +0.51%]
  • ✔️001_tpch_20: 42.1ms -> 42.2ms [-0.56%, +1.12%]
  • ✔️001_tpch_21: 75ms -> 75.8ms [-0.48%, +2.7%]
  • ✔️001_tpch_22: 43.7ms -> 43.9ms [-0.45%, +1.61%]
  • ✔️010_tpch_01: 78.3ms -> 79.1ms [-6.83%, +8.88%]
  • ✔️010_tpch_02: 67.7ms -> 68.2ms [-2.5%, +4%]
  • ✔️010_tpch_03: 54.9ms -> 54.8ms [-1.56%, +0.96%]
  • ✔️010_tpch_04: 41.1ms -> 40.4ms [-4.71%, +1.39%]
  • ✔️010_tpch_05: 85.5ms -> 85.5ms [-1.46%, +1.39%]
  • ✔️010_tpch_06: 29.2ms -> 29.9ms [-6.9%, +12.07%]
  • ✔️010_tpch_07: 101ms -> 98.3ms [-6.41%, +2.04%]
  • ✔️010_tpch_08: 123ms -> 121ms [-3.78%, +1.47%]
  • ✔️010_tpch_09: 112ms -> 111ms [-1.96%, +0.49%]
  • ✔️010_tpch_10: 70.2ms -> 69.5ms [-3.29%, +1.04%]
  • ✔️010_tpch_11: 35.6ms -> 35.7ms [-3.63%, +4.11%]
  • ✔️010_tpch_12: 49.5ms -> 50ms [-1.47%, +3.79%]
  • ✔️010_tpch_13: 49.4ms -> 50.1ms [-0.98%, +3.75%]
  • ✔️010_tpch_14: 33.7ms -> 32.8ms [-5.44%, +0.02%]
  • ✔️010_tpch_15: 50.3ms -> 50.4ms [-6.61%, +7.16%]
  • ✔️010_tpch_16: 37.6ms -> 37.3ms [-3.32%, +1.97%]
  • ✔️010_tpch_17: 52.7ms -> 51.1ms [-8.68%, +2.81%]
  • ✔️010_tpch_18: 52.9ms -> 51.7ms [-8.68%, +4.05%]
  • ✔️010_tpch_19: 84ms -> 82.4ms [-5.38%, +1.77%]
  • ✔️010_tpch_20: 63ms -> 63.7ms [-4.41%, +6.41%]
  • ✔️010_tpch_21: 233ms -> 232ms [-2.8%, +2.17%]
  • ✔️010_tpch_22: 50.8ms -> 50.5ms [-2.87%, +1.78%]
  • ✔️100_tpch_01: 317ms -> 304ms [-23.03%, +15.16%]
  • ✔️100_tpch_02: 126ms -> 124ms [-13.87%, +10.72%]
  • ✔️100_tpch_03: 166ms -> 168ms [-7.85%, +10.3%]
  • ✔️100_tpch_04: 141ms -> 148ms [-5.22%, +15.1%]
  • ✔️100_tpch_05: 253ms -> 251ms [-15.06%, +13.59%]
  • ✔️100_tpch_06: 98.7ms -> 91.7ms [-18.7%, +4.54%]
  • ❗🐌100_tpch_07: 210ms -> 232ms [+1.34%, +20.06%]
  • ✔️100_tpch_08: 263ms -> 247ms [-15.44%, +3.36%]
  • ✔️100_tpch_09: 351ms -> 339ms [-18.78%, +11.7%]
  • ✔️100_tpch_10: 207ms -> 203ms [-6.08%, +2.21%]
  • ✔️100_tpch_11: 81ms -> 80ms [-6.35%, +3.8%]
  • ✔️100_tpch_12: 178ms -> 182ms [-14.83%, +19.16%]
  • 🚀100_tpch_13: 308ms -> 303ms [-2.4%, -0.45%]
  • ✔️100_tpch_14: 111ms -> 107ms [-12.87%, +4.47%]
  • 🚀100_tpch_15: 203ms -> 193ms [-7.8%, -1.87%]
  • ✔️100_tpch_16: 128ms -> 126ms [-16.46%, +13.59%]
  • ✔️100_tpch_17: 188ms -> 187ms [-17.56%, +15.61%]
  • ✔️100_tpch_18: 182ms -> 186ms [-1.94%, +5.36%]
  • ✔️100_tpch_19: 258ms -> 270ms [-4.61%, +13.84%]
  • ✔️100_tpch_20: 171ms -> 163ms [-14.18%, +5.28%]
  • ✔️100_tpch_21: 1.3s -> 1.27s [-5.8%, +2.03%]
  • ✔️100_tpch_22: 152ms -> 152ms [-12.06%, +12.73%]

Further explanation regarding interpretation and methodology can be found in the documentation.

Copy link
Member

@krlmlr krlmlr left a comment

Choose a reason for hiding this comment

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

Thanks!

@@ -245,7 +245,7 @@ In dtplyr and dbplyr, there are no unfunneled frames: collection always needs to
## Partial funneling

Partial funneling is a compromise between funneling and unfunneling.
Materialization is allowed for data up to a certain size, measured in cells (values) or rows in the resulting data frame.
Materialization is allowed for data up to a certain size, measured in cells (values) and rows in the resulting data frame.
Copy link
Member

Choose a reason for hiding this comment

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

🤷

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's a detail but because of this and because of there being only examples with one or the other, I thought it was either or.

Comment on lines +93 to +94
#' The default is to inherit the funneling of the input.
#' see the "Funneling" section.
Copy link
Member

Choose a reason for hiding this comment

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

I forgot that this section needs to go or be shortened drastically, pointing to the vignette.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You made a comment about this but I'll track it in an issue.

@krlmlr krlmlr merged commit 84cf887 into main Jan 28, 2025
7 checks passed
@krlmlr krlmlr deleted the param branch January 28, 2025 11:09
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.

2 participants