-
Notifications
You must be signed in to change notification settings - Fork 21
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Simple form of y-transform #3127
Conversation
🦋 Changeset detectedLatest commit: 03b8164 The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Ignored Deployment
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't read the tests, but tested this in the playground and output seems mostly fine.
Because of EPSILON and numeric issues, the output will be buggy for multiple transforms applied in a row, but I can't find a quick way to fix that.
const x1 = shape.xs[i]; | ||
const x2 = shape.xs[i + 1]; | ||
const y1 = shape.ys[i]; | ||
const y2 = shape.ys[i + 1]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not that fond of overdoing FP, but in this case maybe having a common E_A.forEachPair(() => { ... })
would make sense.
(E_A.forEachPair : Array.forEach :: E_A.pairwise : Array.map
)
|
||
rectangles.push({ | ||
x1: Math.min(y1, y2), | ||
x2: Math.max(y1, y2), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(minor)
I'd probably swap y1
and y2
early on to avoid these min/max calls. Like,
let y1 = ...;
let y2 = ...;
if (y1 > y2) [y1, y2] = [y2, y1];
for (let i = 0; i < keys.length; i++) { | ||
result.push([keys[i], y]); | ||
y += map.get(keys[i])!; | ||
result.push([keys[i], y]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should add Number.EPSILON
; XYShapes with identical xs values are not valid, as we discussed in DMs.
(this is still hacky but we don't have real stepwise XYShapes yet)
This is my code but it works identical as your previous version, and changing this would require test updates, so I'll leave this for you to improve (or keep as is and deal with bugs later; both versions would be buggy, for different reasons).
Co-authored-by: Slava Matyukhin <me@berekuk.ru>
* main: changeset implement Common.try Create khaki-lamps-repair.md fix x = {||...} -> ... parser bug; avoid extra block nodes in AST minor refactorings rename #indexLookup to forbid overrides clean up types remove FRFunction.output Create early-moons-return.md support != in lezer grammar (fixes #3071) upgrade to turbo 1.13 ts fixes for latest pothos fix DefaultAuthStrategy Minor cleanup Dist chart can now show negative values First pass at handling negative dists ⬆️ Bump @pothos/plugin-prisma from 3.63.1 to 3.64.0
Experimentation:
Or maybe:
My impression is that it's doing okay for continuous, but breaking for discrete.