Skip to content

Resubmission: Update knot vector synthesis for approximate_curve and approximate_surface #187

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

Open
wants to merge 3 commits into
base: 5.x
Choose a base branch
from

Conversation

GarthSnyder
Copy link

@GarthSnyder GarthSnyder commented May 21, 2025

This is a replacement for PR #121, which ended up having a conflicting branch name. It doesn't look like it's possible to substitute the submitted branch within the GitHub system.

This PR fixes #119 and #120. It includes tests to verify that knot vectors are well-formed, that every knot span includes at least one new control point, and that knot vectors generated for reversed curves are the inverses of those generated in the opposite direction.

@GarthSnyder GarthSnyder changed the title Update knot vector synthesis for approximate_curve and approximate_surface Resubmission: Update knot vector synthesis for approximate_curve and approximate_surface May 21, 2025
@orbingol
Copy link
Owner

orbingol commented May 22, 2025

Thank you very much for resubmitting the PR. I've merged some changes to the 5.x branch, so this PR might need an update. Tests are failing due to some changes I merged from 5.x-dev branch. After fixing them, I'll take a look at this PR.

@orbingol orbingol self-assigned this May 22, 2025
@orbingol
Copy link
Owner

Also, you might need to install pre-commit (e.g. pip install pre-commit) and run via pre-commit run --all-files to apply the suggestions of the configured code formatter. This is not documented at this time, just wanted to explicitly mention about it.

@GarthSnyder
Copy link
Author

Thanks, I will make a detailed review. I'm away for the Memorial Day weekend but will look at this next week.

@orbingol orbingol removed their assignment May 23, 2025
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.

Division by zero in backward_substitution while fitting spline
2 participants