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

enhance(scatter): remove entities from the data table that are not displayed in the chart #2720

Merged
merged 3 commits into from
Nov 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ export interface ChartInterface {
// Todo: should all charts additionally have a placedSeries: ChartPlacedSeries[] getter?

transformTable: ChartTableTransformer
transformTableForDisplay?: ChartTableTransformer

yAxis?: HorizontalAxis | VerticalAxis
xAxis?: HorizontalAxis | VerticalAxis
Expand Down
9 changes: 9 additions & 0 deletions packages/@ourworldindata/grapher/src/core/Grapher.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -658,6 +658,15 @@ export class Grapher
return this.xAxis.toObject()
}

// table that is used for display in the table tab
@computed get tableForDisplay(): OwidTable {
const table = this.table
if (!this.isReady || !this.isOnTableTab) return table
return this.chartInstance.transformTableForDisplay
? this.chartInstance.transformTableForDisplay(table)
: table
}

@computed get tableForSelection(): OwidTable {
// This table specifies which entities can be selected in the charts EntitySelectorModal.
// It should contain all entities that can be selected, and none more.
Expand Down
12 changes: 9 additions & 3 deletions packages/@ourworldindata/grapher/src/dataTable/DataTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,8 @@ const inverseSortOrder = (order: SortOrder): SortOrder =>
order === SortOrder.asc ? SortOrder.desc : SortOrder.asc

export interface DataTableManager {
table: OwidTable
table: OwidTable // not used here, but required in type `ChartManager`
tableForDisplay: OwidTable
entityType?: string
endTime?: Time
startTime?: Time
Expand Down Expand Up @@ -129,7 +130,7 @@ export class DataTable extends React.Component<{
}

@computed get table(): OwidTable {
let table = this.manager.table
let table = this.manager.tableForDisplay
if (this.manager.showSelectionOnlyInDataTable) {
table = table.filterByEntityNames(
this.selectionArray.selectedEntityNames
Expand All @@ -139,7 +140,12 @@ export class DataTable extends React.Component<{
}

@computed get manager(): DataTableManager {
return this.props.manager ?? { table: BlankOwidTable() }
return (
this.props.manager ?? {
table: BlankOwidTable(),
tableForDisplay: BlankOwidTable(),
}
)
}

@computed private get entityType(): string {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,27 @@ export class ScatterPlotChart
series: ScatterSeries
}>()

private filterManuallySelectedEntities(table: OwidTable): OwidTable {
const { includedEntities, excludedEntities } = this.manager
const excludedEntityIdsSet = new Set(excludedEntities)
const includedEntityIdsSet = new Set(includedEntities)
const excludeEntitiesFilter = (entityId: any): boolean =>
!excludedEntityIdsSet.has(entityId as number)
const includedEntitiesFilter = (entityId: any): boolean =>
includedEntityIdsSet.size > 0
? includedEntityIdsSet.has(entityId as number)
: true
const filterFn = (entityId: any): boolean =>
excludeEntitiesFilter(entityId) && includedEntitiesFilter(entityId)
const excludedList = excludedEntities ? excludedEntities.join(", ") : ""
const includedList = includedEntities ? includedEntities.join(", ") : ""
return table.columnFilter(
OwidTableSlugs.entityId,
filterFn,
`Excluded entity ids specified by author: ${excludedList} - Included entity ids specified by author: ${includedList}`
)
}

transformTable(table: OwidTable): OwidTable {
const {
backgroundSeriesLimit,
Expand All @@ -138,28 +159,7 @@ export class ScatterPlotChart
}

if (excludedEntities || includedEntities) {
const excludedEntityIdsSet = new Set(excludedEntities)
const includedEntityIdsSet = new Set(includedEntities)
const excludeEntitiesFilter = (entityId: any): boolean =>
!excludedEntityIdsSet.has(entityId as number)
const includedEntitiesFilter = (entityId: any): boolean =>
includedEntityIdsSet.size > 0
? includedEntityIdsSet.has(entityId as number)
: true
const filterFn = (entityId: any): boolean =>
excludeEntitiesFilter(entityId) &&
includedEntitiesFilter(entityId)
const excludedList = excludedEntities
? excludedEntities.join(", ")
: ""
const includedList = includedEntities
? includedEntities.join(", ")
: ""
table = table.columnFilter(
OwidTableSlugs.entityId,
filterFn,
`Excluded entity ids specified by author: ${excludedList} - Included entity ids specified by author: ${includedList}`
)
table = this.filterManuallySelectedEntities(table)
}

// Allow authors to limit the # of background entities to get better perf and clearer charts.
Expand Down Expand Up @@ -265,6 +265,28 @@ export class ScatterPlotChart
return table
}

transformTableForDisplay(table: OwidTable): OwidTable {
const { includedEntities, excludedEntities } = this.manager

if (excludedEntities || includedEntities) {
table = this.filterManuallySelectedEntities(table)
}

// Drop any rows which have non-number values for X or Y.
table = table
.columnFilter(
this.xColumnSlug,
isNumber,
"Drop rows with non-number values in X column"
)
.columnFilter(
this.yColumnSlug,
isNumber,
"Drop rows with non-number values in Y column"
)
return table
}

@computed get inputTable(): OwidTable {
return this.manager.table
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -255,35 +255,35 @@ export class MarimekkoChart
entityName: string
}>()

private filterManuallySelectedEntities(table: OwidTable): OwidTable {
const { excludedEntities, includedEntities } = this.manager
const excludedEntityIdsSet = new Set(excludedEntities)
const includedEntityIdsSet = new Set(includedEntities)
const excludeEntitiesFilter = (entityId: any): boolean =>
!excludedEntityIdsSet.has(entityId as number)
const includedEntitiesFilter = (entityId: any): boolean =>
includedEntityIdsSet.size > 0
? includedEntityIdsSet.has(entityId as number)
: true
const filterFn = (entityId: any): boolean =>
excludeEntitiesFilter(entityId) && includedEntitiesFilter(entityId)
const excludedList = excludedEntities ? excludedEntities.join(", ") : ""
const includedList = includedEntities ? includedEntities.join(", ") : ""
return table.columnFilter(
OwidTableSlugs.entityId,
filterFn,
`Excluded entity ids specified by author: ${excludedList} - Included entity ids specified by author: ${includedList}`
)
}

transformTable(table: OwidTable): OwidTable {
const { excludedEntities, includedEntities } = this.manager
const { yColumnSlugs, manager, colorColumnSlug, xColumnSlug } = this
if (!this.yColumnSlugs.length) return table

if (excludedEntities || includedEntities) {
const excludedEntityIdsSet = new Set(excludedEntities)
const includedEntityIdsSet = new Set(includedEntities)
const excludeEntitiesFilter = (entityId: any): boolean =>
!excludedEntityIdsSet.has(entityId as number)
const includedEntitiesFilter = (entityId: any): boolean =>
includedEntityIdsSet.size > 0
? includedEntityIdsSet.has(entityId as number)
: true
const filterFn = (entityId: any): boolean =>
excludeEntitiesFilter(entityId) &&
includedEntitiesFilter(entityId)
const excludedList = excludedEntities
? excludedEntities.join(", ")
: ""
const includedList = includedEntities
? includedEntities.join(", ")
: ""
table = table.columnFilter(
OwidTableSlugs.entityId,
filterFn,
`Excluded entity ids specified by author: ${excludedList} - Included entity ids specified by author: ${includedList}`
)
} else table = table
table = this.filterManuallySelectedEntities(table)
}

// TODO: remove this filter once we don't have mixed type columns in datasets
table = table.replaceNonNumericCellsWithErrorValues(yColumnSlugs)
Expand Down Expand Up @@ -329,6 +329,16 @@ export class MarimekkoChart
return table
}

transformTableForDisplay(table: OwidTable): OwidTable {
const { includedEntities, excludedEntities } = this.manager

if (excludedEntities || includedEntities) {
table = this.filterManuallySelectedEntities(table)
}

return table
}

@computed get inputTable(): OwidTable {
return this.manager.table
}
Expand Down
Loading