Skip to content
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

docs update tofor canvas.stroke(path2D) in drawing_paths.rst #356

Closed
wants to merge 12 commits into from

Conversation

cleemesser
Copy link
Contributor

Same docs for drawing paths as discussed

Copy link
Contributor

github-actions bot commented Sep 7, 2024

lite-badge 👈 Try it on ReadTheDocs with JupyterLite!

@cleemesser
Copy link
Contributor Author

@martinRenou Oops, I probably have a git workflow mistake. I did not expect the PR to have all these different commits. I think I made a mistake pushing to my own master branch first. I I assume you squash those out when it comes time to accept. In the future, I'm guessing I should delete my current fork, and re-fork and do my PRs from a branch.

@cleemesser
Copy link
Contributor Author

Hmm, I just noticed the sinusoidal stroke is not being rendered in the read the docs output for drawing paths

Is that a bug in my submission to ipycanvas itself? Or, is it in the the build process not updating to the latest package version? Let me know if there is something I can help fix.

@martinRenou
Copy link
Collaborator

I made a mistake pushing to my own master branch first

Indeed this is the reason you see all these commits. On my end I'm fine squashing.

In the future, I'm guessing I should delete my current fork, and re-fork and do my PRs from a branch.

Probably that should be best. Alternatively you can remove your master branch and force update your fork (don't do it quite yet as it would remove this PR):

git branch -D master ## Probably need to run this from another branch than master
git fetch https://github.com/jupyter-widgets-contrib/ipycanvas master
git checkout master
git push -f https://github.com/cleemesser/ipycanvas master

Is that a bug in my submission to ipycanvas itself?

There is probably an action I should take on the publishing to fix this.

@@ -6,7 +6,7 @@ There are two ways for creating and drawing a path in ipycanvas.
Using Path2D
------------

You can define a Path2D given an SVG path. Note that once the path is created, it is read only, you cannot dynamically change the path value. This means they do not (yet) support Path2D methods like Path2D.move_to, draw_line, etc.
You can define a Path2D given an SVG path. Note that once the path is created, it is read only, you cannot dynamically change the path value. ``ipycanvas`` does not (yet) support Path2D methods like Path2D.move_to, draw_line, etc.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Was this based off my review from your other PR? I actually prefer the original version you came up with, I changed my mind

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I read that discussion and made the change. If you changed your mind, you can close this PR, and I will fix my fork.
Thanks for the git help.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sure thing! Thanks for your PRs!

@cleemesser cleemesser closed this Sep 10, 2024
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.

2 participants