Skip to content

fix(maths): scale-minmax-table fails on current Nu (undefined column2, deprecated merge)#1265

Merged
fdncred merged 1 commit into
nushell:mainfrom
tonythethompson:fix/math-functions-scale-minmax-table
Jul 2, 2026
Merged

fix(maths): scale-minmax-table fails on current Nu (undefined column2, deprecated merge)#1265
fdncred merged 1 commit into
nushell:mainfrom
tonythethompson:fix/math-functions-scale-minmax-table

Conversation

@tonythethompson

Copy link
Copy Markdown
Contributor

What's broken

scale-minmax-table in modules/maths/math_functions.nu fails on current Nu (tested on 0.113.1) for two independent reasons:

  1. column2 is undefined in this module. It's only defined in sourced/misc/nu_defs.nu (an unrelated personal-dotfiles file elsewhere in this repo). scale-minmax-table calls it without importing or defining it locally, so it only ever worked in a shell session where nu_defs.nu happened to already be sourced separately. As a standalone module command, it's always been broken.
  2. merge {$it} is no longer valid. merge used to accept a closure form; current Nu's merge takes a record/table value directly (see help merge). This fails to parse on recent Nu regardless of the column2 issue.

Fix

  • Add a local column2 helper (same implementation as the one in sourced/misc/nu_defs.nu) so this module doesn't depend on an unrelated file being sourced first.
  • merge {$it} -> merge $it.

How I found this

I was evaluating this module as a candidate seed package for an unrelated project's package registry and ran it against a real Nu 0.113.1 instead of trusting the source — it failed to parse. Diagnosed both root causes from there.

Verification

On Nu 0.113.1:

  • The whole module now parses cleanly (it did not before).
  • [[a b]; [1 10] [2 20] [3 30]] | scale-minmax-table 0 1 produces correct per-column min-max scaling:
    ╭───┬──────┬──────╮
    │ # │  a   │  b   │
    ├───┼──────┼──────┤
    │ 0 │ 0.00 │ 0.00 │
    │ 1 │ 0.50 │ 0.50 │
    │ 2 │ 1.00 │ 1.00 │
    ╰───┴──────┴──────╯
    
  • Spot-checked root, croot, and delta in the same file still produce correct output, unaffected by this change.

…, deprecated merge)

scale-minmax-table has been broken as a standalone module command since
it was added: it calls `column2`, which is defined only in
sourced/misc/nu_defs.nu (an unrelated personal-dotfiles file) rather
than in this module. It only ever worked in a shell where that file
happened to already be sourced separately.

Separately, current Nu's `merge` no longer accepts the closure form
`merge {$it}` used here -- `merge` now takes a `record`/`table` value
directly (see `help merge`), so this also fails to parse on anything
recent regardless of the column2 issue.

Fixes:
- Add a local `column2` helper (same implementation as
  sourced/misc/nu_defs.nu) so the module doesn't depend on unrelated
  files being sourced first.
- `merge {$it}` -> `merge $it`.

Verified on Nu 0.113.1: the whole module now parses, and
`[[a b]; [1 10] [2 20] [3 30]] | scale-minmax-table 0 1` produces the
correct per-column min-max scaling. Also spot-checked root, croot, and
delta still work unchanged.
Copilot AI review requested due to automatic review settings July 2, 2026 03:43

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Fixes scale-minmax-table in the maths module so it works on current Nushell by removing an implicit dependency on an unrelated sourced file and updating a now-invalid merge syntax.

Changes:

  • Added a local (non-exported) column2 helper to make scale-minmax-table self-contained.
  • Updated merge {$it} to merge $it to match current Nushell merge parsing/usage.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@fdncred

fdncred commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Thanks!

@fdncred fdncred merged commit 4af42d7 into nushell:main Jul 2, 2026
1 check passed
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.

3 participants