Skip to content

Commit

Permalink
experiment: remove parent in core-table
Browse files Browse the repository at this point in the history
  • Loading branch information
marcelgerber committed Feb 28, 2025
1 parent 680d1f4 commit ebf5cea
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 73 deletions.
18 changes: 0 additions & 18 deletions packages/@ourworldindata/core-table/src/CoreTable.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -510,23 +510,13 @@ describe("searching", () => {
expect(table.grep("200").numRows).toEqual(2)
expect(table.grep(/20\d+/).numRows).toEqual(2)

expect(
table.grep("Germany").grep("2001").opposite.rows[0].year
).toEqual(2000)
expect(table.grep("Germany").opposite.numRows).toEqual(1)
expect(table.grep(/(1999|2000)/).numRows).toEqual(2)
})

it("can filter columns as well", () => {
expect(table.grepColumns("country").numColumns).toEqual(1)
expect(table.grepColumns("r").numColumns).toEqual(2)
expect(table.grepColumns("zz").numColumns).toEqual(0)
expect(table.grepColumns("year").oppositeColumns.columnSlugs).toEqual([
"country",
])
expect(table.grepColumns(/co.+/).oppositeColumns.columnSlugs).toEqual([
"year",
])
})

it("can get the domain across all columns", () => {
Expand Down Expand Up @@ -666,14 +656,6 @@ describe("filtering", () => {
})
})

describe("debug tools", () => {
const table = new CoreTable(sampleCsv).dropColumns(["population"])

it("can dump its ancestors", () => {
expect(table.ancestors.length).toEqual(2)
})
})

describe("printing", () => {
it("uses slugs for headers in toDelimited", () => {
const table = new CoreTable(`country,Population in 2020
Expand Down
67 changes: 12 additions & 55 deletions packages/@ourworldindata/core-table/src/CoreTable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ import { applyTransforms, extractTransformNameAndParams } from "./Transforms.js"
interface AdvancedOptions {
tableDescription?: string
transformCategory?: TransformType
parent?: CoreTable
hasParent?: boolean
filterMask?: FilterMask
tableSlug?: TableSlug
}
Expand All @@ -85,7 +85,7 @@ export class CoreTable<
COL_DEF_TYPE extends CoreColumnDef = CoreColumnDef,
> {
private _columns: Map<ColumnSlug, CoreColumn> = new Map()
protected parent?: this
protected hasParent: boolean
tableDescription: string
private timeToLoad = 0
private initTime = Date.now()
Expand All @@ -100,11 +100,11 @@ export class CoreTable<
advancedOptions: AdvancedOptions = {}
) {
const start = Date.now() // Perf aid
const { parent, tableDescription = "" } = advancedOptions
const { tableDescription = "", hasParent } = advancedOptions

this.originalInput = input
this.tableDescription = tableDescription
this.parent = parent as this
this.hasParent = hasParent ?? false
this.inputColumnDefs =
typeof inputColumnDefs === "string"
? columnDefinitionsFromInput<COL_DEF_TYPE>(inputColumnDefs)
Expand Down Expand Up @@ -141,7 +141,7 @@ export class CoreTable<

// If this has a parent table, than we expect all defs. This makes "deletes" and "renames" fast.
// If this is the first input table, then we do a simple check to generate any missing column defs.
if (!parent)
if (!advancedOptions.hasParent)
autodetectColumnDefs(this.inputColumnStore, this._columns).forEach(
(def) => this.setColumn(def as COL_DEF_TYPE)
)
Expand Down Expand Up @@ -318,15 +318,15 @@ export class CoreTable<
)
}

if (this.parent || !firstInputRow) return []
if (this.hasParent || !firstInputRow) return []

// The default behavior is to assume some missing or bad data in user data, so we always parse the full input the first time we load
// user data, with the exception of columns that have values passed directly.
// Todo: measure the perf hit and add a parameter to opt out of this this if you know the data is complete?
const alreadyTypedSlugs = new Set(
Object.keys(this.valuesFromColumnDefs)
)
if (this.isRoot) {
if (!this.hasParent) {
return columnsToMaybeParse.filter(
(col) => !alreadyTypedSlugs.has(col.slug)
)
Expand Down Expand Up @@ -359,7 +359,7 @@ export class CoreTable<
// The combo of the "this" return type and then casting this to any allows subclasses to create transforms of the
// same type. The "any" typing is very brief (the returned type will have the same type as the instance being transformed).
return new (this.constructor as any)(rowsOrColumnStore, defs, {
parent: this,
hasParent: true,
tableDescription,
transformCategory,
filterMask,
Expand All @@ -370,9 +370,7 @@ export class CoreTable<
// A large time may just be due to a transform only happening after a user action, or it
// could be do to other sync code executing between transforms.
private get betweenTime(): number {
return this.parent
? this.initTime - (this.parent.initTime + this.parent.timeToLoad)
: 0
return 0 // TODO
}

@imemo get rows(): ROW_TYPE[] {
Expand Down Expand Up @@ -507,7 +505,7 @@ export class CoreTable<
}

get rootTable(): this {
return this.parent ? this.parent.rootTable : this
return this // TODO
}

/**
Expand Down Expand Up @@ -572,33 +570,6 @@ export class CoreTable<
}, `Kept rows that matched '${searchStringOrRegex.toString()}'`)
}

get opposite(): this {
const { parent } = this
const { filterMask } = this.advancedOptions
if (!filterMask || !parent) return this
return this.transform(
parent.columnStore,
this.defs,
`Inversing previous filter`,
TransformType.InverseFilterRows,
filterMask.inverse()
)
}

@imemo get oppositeColumns(): this {
if (this.isRoot) return this
const columnsToDrop = new Set(this.columnSlugs)
const defs = this.parent!.columnsAsArray.filter(
(col) => !columnsToDrop.has(col.slug)
).map((col) => col.def) as COL_DEF_TYPE[]
return this.transform(
this.columnStore,
defs,
`Inversing previous column filter`,
TransformType.InverseFilterColumns
)
}

grepColumns(searchStringOrRegex: string | RegExp): this {
const columnsToDrop = this.columnSlugs.filter((slug) => {
return typeof searchStringOrRegex === "string"
Expand Down Expand Up @@ -751,21 +722,11 @@ export class CoreTable<
)
}

private get isRoot(): boolean {
return !this.parent
}

dump(rowLimit = 30): void {
this.dumpPipeline()
this.dumpColumns()
this.dumpRows(rowLimit)
}

dumpPipeline(): void {
// eslint-disable-next-line no-console
console.table(this.ancestors.map((tb) => tb.explanation))
}

dumpColumns(): void {
// eslint-disable-next-line no-console
console.table(this.explainColumns)
Expand Down Expand Up @@ -841,10 +802,6 @@ export class CoreTable<
})
}

get ancestors(): this[] {
return this.parent ? [...this.parent.ancestors, this] : [this]
}

@imemo private get numColsToParse(): number {
return this.colsToParse.length
}
Expand Down Expand Up @@ -973,7 +930,7 @@ export class CoreTable<
return this.concat(
[
new (this.constructor as typeof CoreTable)(rows, this.defs, {
parent: this,
hasParent: true,
}),
],
opDescription
Expand Down Expand Up @@ -1529,7 +1486,7 @@ export class CoreTable<
const appendTable = new (this.constructor as typeof CoreTable)(
appendColumnStore,
this.defs,
{ parent: this }
{ hasParent: true }
)

return this.concat(
Expand Down

0 comments on commit ebf5cea

Please sign in to comment.