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

🎉 support entity exclusion for all chart types #4583

Merged
merged 1 commit into from
Feb 26, 2025

Conversation

sophiamersmann
Copy link
Member

@sophiamersmann sophiamersmann commented Feb 20, 2025

Allows to exclude entities for all chart types (used to be supported for Scatter and Marimekko only).

Entity exclusion is similar to the author's timeline filter in the sense that it should seem like the excluded entities simply don't exist, so the entity exclusion runs as early as the author timeline filter.

@owidbot
Copy link
Contributor

owidbot commented Feb 20, 2025

Quick links (staging server):

Site Dev Site Preview Admin Wizard Docs

Login: ssh owid@staging-site-grapher-exclude-entities

SVG tester:

Number of differences (default views): 0 ✅
Number of differences (all views): 0 ✅

Edited: 2025-02-26 11:09:54 UTC
Execution time: 1.35 seconds

@sophiamersmann sophiamersmann changed the title 🎉 early entity exclusion for all chart types 🎉 add entity exclusion for all chart types Feb 24, 2025
@sophiamersmann sophiamersmann force-pushed the grapher-exclude-entities branch from 3dcae7a to effd297 Compare February 24, 2025 10:05
@@ -865,7 +865,7 @@ export class Grapher
// (2) needs to have data for the x and y dimension.
let table = this.isScatter
? this.tableAfterAuthorTimelineAndActiveChartTransform
: this.inputTable
: this.table
Copy link
Member Author

@sophiamersmann sophiamersmann Feb 24, 2025

Choose a reason for hiding this comment

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

Since table is the cleaned inputTable that we actually want to use as a base table for Grapher, I think it makes sense to use it for the entity selector as well.

Copy link
Member Author

sophiamersmann commented Feb 26, 2025

@sophiamersmann sophiamersmann force-pushed the grapher-exclude-entities branch from 403876a to 72228a4 Compare February 26, 2025 09:37
@sophiamersmann sophiamersmann changed the base branch from refactor-entity-ids to graphite-base/4583 February 26, 2025 10:40
@sophiamersmann sophiamersmann force-pushed the grapher-exclude-entities branch from 72228a4 to 86cbd16 Compare February 26, 2025 10:40
@sophiamersmann sophiamersmann changed the base branch from graphite-base/4583 to master February 26, 2025 10:41
@sophiamersmann sophiamersmann force-pushed the grapher-exclude-entities branch from 86cbd16 to bc428b6 Compare February 26, 2025 10:41
@sophiamersmann sophiamersmann marked this pull request as ready for review February 26, 2025 11:19
@sophiamersmann sophiamersmann force-pushed the grapher-exclude-entities branch from bc428b6 to 29ef92e Compare February 26, 2025 11:19
@sophiamersmann sophiamersmann changed the title 🎉 add entity exclusion for all chart types 🎉 support entity exclusion for all chart types Feb 26, 2025
Copy link
Member

@marcelgerber marcelgerber left a comment

Choose a reason for hiding this comment

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

awesome!

and yeah, I agree on changing the basis for tableForSelection, makes sense.

Copy link
Member Author

sophiamersmann commented Feb 26, 2025

Merge activity

  • Feb 26, 8:49 AM EST: A user started a stack merge that includes this pull request via Graphite.
  • Feb 26, 8:50 AM EST: A user merged this pull request with Graphite.

@sophiamersmann sophiamersmann merged commit 33f00fe into master Feb 26, 2025
31 checks passed
@sophiamersmann sophiamersmann deleted the grapher-exclude-entities branch February 26, 2025 13:50
Copy link

sentry-io bot commented Mar 3, 2025

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ RangeError: Out of memory _a.columnFilter(packages/@ourworldindata/core-t... View Issue

Did you find this useful? React with a 👍 or 👎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
staging-viz Let SVG tester fail silently in CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants