fix(Regression): preserve time scale and activate fx/fy faceting#549
Merged
fix(Regression): preserve time scale and activate fx/fy faceting#549
Conversation
Two bugs fixed in RegressionY/RegressionX:
1. Time scale regression: toOutputX was reading plot.scales[independent].type
to decide whether to emit Date objects for the inner Line mark. On the first
render pass the scale type is not yet resolved, so numeric values were emitted
instead. Scale inference then saw a mix of Dates (from the user's marks) and
numbers (from Regression's Line) and permanently fell back to 'linear'.
Fix: detect temporality from the raw input data (instanceof Date) instead of
the scale type, breaking the circular dependency.
2. fx/fy faceting: the outer <Mark type="regression"> had no data and no fx/fy
props, so it contributed nothing to the fx/fy scale domain. The inner <Line>
also had fx:null/fy:null. With no mark populating the domain, FacetGrid saw
an empty domain and never split into facet cells.
Fix: pass {data}, fx, and fy to the outer <Mark> in RegressionY and
RegressionX so scale computation collects the facet domain values.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
✅ Deploy Preview for svelteplot ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
📦 Preview package for this PR is published! Version: Install it with: npm install svelteplot@pr-549
# or install the specific version
npm install svelteplot@0.14.1-pr-549.0 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b5cb58fa3a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…irst independentIsDate was checking only filteredData[0]. A leading row with a null/undefined/invalid independent-channel value returns null from resolveChannel, not a Date, so the flag would be false even when later rows are Dates. This caused regrData to emit numeric __x values, which could push scale inference back to 'linear' when Regression is the only contributing mark. Switch to Array.some() so we find the first row that actually resolves to a Date, regardless of position. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
📦 Preview package for this PR is published! Version: Install it with: npm install svelteplot@pr-549
# or install the specific version
npm install svelteplot@0.14.1-pr-549.1 |
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
📦 Preview package for this PR is published! Version: Install it with: npm install svelteplot@pr-549
# or install the specific version
npm install svelteplot@0.14.1-pr-549.2 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Time scale broken by RegressionY/X:
Regression.sveltewas callingtoOutputX(value, plot.scales[independent].type)to decide whether to convert epoch-ms numbers back toDateobjects for the innerLinemark. On the first render pass the scale type isn't resolved yet ("linear"), so numbers were emitted. Scale inference then saw a mix ofDatevalues (from the user's marks) and numbers (from Regression'sLine) and permanently fell back to"linear", destroying nice time-axis ticks. Fix: derive temporality fromfilteredData[0](instanceof Date) instead of the scale type, breaking the circular dependency.fx/fy faceting not activated: the outer
<Mark type="regression">inRegressionY/RegressionXhad nodataand nofx/fyprops, and the inner<Line>explicitly setfx: null, fy: null. Neither contributed to the fx/fy scale domain.FacetGridchecksplot.scales.fx.domain.length > 0to decide whether to split into facet cells — with an empty domain it never did. Fix: pass{data},fx={options.fx}, andfy={options.fy}to the outer<Mark>in bothRegressionYandRegressionX.Test plan
pnpm test— all 834 tests pass (added regression tests for both bugs)preserves time scale when x channel contains Date values— verifies regression path is valid and NaN-free on a time x-axisactivates fx faceting when fx channel is set— verifies twog.facetelements are created and each contains a regression line🤖 Generated with Claude Code