Skip to content

refactor: consolidate _from_records_build_*_col into a single parametric builder #749

Description

@JRedrupp

Problem

_frame.mojo contains five nearly-identical functions (lines 1316–1457):

  • _from_records_build_null_col
  • _from_records_build_string_col
  • _from_records_build_float_col
  • _from_records_build_int_col
  • _from_records_build_bool_col

All follow the same template:

  1. Allocate typed output list + NullMask
  2. Loop over vals: List[DFScalar]
  3. For each value: if null → append sentinel + append_null(); else coerce to target type + append_valid()
  4. Build Column from typed list + dtype
  5. Attach null mask and return

Only the target type, default sentinel value, and coercion logic differ between them.

Fix

Now that the visitor pattern is being retired (#747), these can be unified as direct isa[]() dispatch helpers or via a small parametric template. The type-inference call site at line 1800 already handles the "which builder to call" decision — the builders themselves should collapse to one implementation.

~130 lines → ~30.

Metadata

Metadata

Assignees

No one assigned

    Labels

    refactorCode refactoring opportunitiestech-debtTechnical debt items

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions