-
Notifications
You must be signed in to change notification settings - Fork 14
Update examples.md #137
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
Merged
Merged
Update examples.md #137
Changes from 1 commit
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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.
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.
missing link?
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.
added link.
Do you think these examples should also be updated ? - beyond my expertise sorry.
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 am not sure we changed so much up to the point we need updates but if sme example is badly broken I'd rather try to fix it or understand why is that, if that makes sense.
Uh oh!
There was an error while loading. Please reload this page.
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.
Four primary issues you might want to revisit:
Icosahedron example
Simple slider panel
KMeans in a panel
matplot example
I will update this PR to remove the 'simple clock' you have since removed so it doesn't reappear if this PR is accepted.
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.
If the examples are fixed - I would be very pleased to update this PR to meet your satisfaction.
Uh oh!
There was an error while loading. Please reload this page.
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'll check these later today - Thanks for doing this Andrea. I really appreciate it.
wrt the Kmeans demo
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.
imho we should improve somehow expectations, at least on PyScript .com side ... with that demo all I see is the inability for ourselves, or our users, to pin a config related dependency on both the Python and the browser side. I am sure it wouldn't be trivial to solve this issue, and we have a way to pin dependencies, but unfortunately that way doesn't necessarily play well with Pyodide registry (not their fault neither). Having a way to keep examples updated is one thing we should consider but having a way to help users keeping their own examples updated seem like a great goal too.
I will discuss this with @ntoll once he's back, but it's clear to me current strategy is not necessarily the best we could offer.
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.
my notes:
Icosahderon
simple panel
matplotlib
kmeans
future: <PyodideTask finished name='Task-41' coro=<write() done,
defined at /lib/python3.12/site-packages/panel/io/pyodide.py:489> exception=Error:
NumberFormatter(p1053).text_align given invalid value: {"value":"left"}>
Uh oh!
There was an error while loading. Please reload this page.
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.
PR updated. Also added new PR to fix icosahedron example on pyscript-examples:
I.e. better lighting
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.
pretty much ... it's Simple for a reason 😁
I have no idea about that example or the underling library but I wonder if updating all scripts to their latest version on the HTML side would just make it work. If it does, then we should likely also pin those versions in the TOML side of affair so that we won't have more surprises in the future.