Skip to content

standalone Plot.scale respects the transform and percent options - #2458

Open
eastagiletracker wants to merge 1 commit into
observablehq:mainfrom
eastagiletracker:agile-board/standalone-scale-transform
Open

standalone Plot.scale respects the transform and percent options#2458
eastagiletracker wants to merge 1 commit into
observablehq:mainfrom
eastagiletracker:agile-board/standalone-scale-transform

Conversation

@eastagiletracker

Copy link
Copy Markdown

This PR proposes making the standalone Plot.scale respect the transform and percent scale options, so that it materializes the same options as plot.scale (fixes #2325). We include this PR work along with a full history of your repo at https://eastagiletracker.com/projects/456. You can sign in with your GitHub ID to claim ownership of the project.

The defect

createScales populates the generic scale options — percent and transform — onto the scale descriptor after createScale returns, which is why plot(…).scale(name) exposes them. The standalone Plot.scale goes straight from normalizeScale to exposeScale and never runs that step, so both options are silently dropped from the returned scale. At 356f579 on main:

$ node --input-type=module -e '
import * as Plot from "./src/index.js";
const transform = (d) => d * 2;
console.log("transform:", Plot.scale({x: {type: "linear", domain: [0, 10], transform}}).transform);
console.log("percent:", Plot.scale({x: {type: "linear", domain: [0, 10], percent: true}}).percent);
'
transform: undefined
percent: undefined

This is not only cosmetic on the returned object. The scales documentation says a standalone scale object can be passed to Plot.plot as the corresponding scale options, and applyScaleTransform in src/plot.js reads transform and percent back out of those options — so a plot that reuses a standalone scale renders untransformed data, while the identical inline options transform it. That asymmetry is what #2325 reports.

The change

The percent and transform handling moves out of createScales into a small applyScaleOptions helper that both createScales and scale call, so the two paths materialize scale options identically instead of one of them re-implementing the other. Plot.plot behavior is unchanged: the helper keeps the existing ordering (percent is applied before the label is inferred, so the “(%)” label suffix still works) and the existing invalid scale transform; not a function error, which the standalone path now raises as well.

Verification

test/scales/scales-test.js gains four tests: the exposed transform, the exposed percent, the not-a-function transform error, and an end-to-end check that a plot handed a standalone scale renders byte-identical output to the same plot handed the equivalent inline options. All four fail on main and pass here.

$ pnpm run test        # main @ 356f579
Test Files  404 passed (404)
     Tests  1349 passed | 2 skipped (1351)
Type Errors  no errors

$ pnpm run test        # this branch
Test Files  404 passed (404)
     Tests  1353 passed | 2 skipped (1355)
Type Errors  no errors

ESLint and Prettier are clean on both trees, and the 28 existing snapshots carrying a “(%)” axis label are unchanged, which is what covers the reordered label inference. One note on merge order: #2427 edits the same line of scale, so whichever of the two lands second needs a one-line rebase.

How this was managed

We ran this work on a board imported from this repository's own issues and pull requests (1875 stories). The story covering this fix is Plot.scale(…) ignores the transform option, on the board at https://eastagiletracker.com/projects/456.

board

If you'd rather not receive contributions like this, reply no-more-prs on this pull request and we won't open any further ones on your repositories.


Lawrence W. Sinclair
CEO / East Agile
linkedin.com/in/lwsinclair/
eastagile.com

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.

Plot.scale(…) ignores the transform option

1 participant