-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Upgrade to Pyodide 0.23 #1347
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
Upgrade to Pyodide 0.23 #1347
Changes from 1 commit
Commits
Show all changes
24 commits
Select commit
Hold shift + click to select a range
ce31e38
Upgrade to Pyodide 0.23.0
JeffersGlass 7bccf09
Update cached files listing
JeffersGlass 2221e12
changelog
JeffersGlass 291c6ff
Address hood's comments
JeffersGlass feba2eb
Address more comments
JeffersGlass 87ca3cf
Update timeout lengths
JeffersGlass 7682bed
Bump pyodide to 0.23.2
JeffersGlass b42cf5a
Merge branch 'main' into pyodide-0-23-0
JeffersGlass 74edd00
Bump kmeans timeout time
JeffersGlass 92e385c
Use @parma decorator to get brush param
JeffersGlass 870cd06
Add a comment to force CI run
JeffersGlass 6c136b3
Separate example tests to run sequentially
JeffersGlass c3befef
Remove extraneous example build
JeffersGlass 335810d
Remove unnecessary 'make examples'
JeffersGlass e36a2b3
Merge branch 'main' into pyodide-0-23-0
JeffersGlass 897af7b
Adjust makefile and rerun CI
JeffersGlass 9197a3f
Remove pytest.raises from test, use check_js_errors
JeffersGlass 413aefa
Add 'check_js_errors' to wait_for_pyscript
JeffersGlass 0443dfc
Cleanup PR
JeffersGlass d91610c
Address some comments
JeffersGlass e7d5190
Adjust comment
JeffersGlass 3e7114d
Rollback change to version check?
JeffersGlass c3ae4bd
Add comment on version
JeffersGlass f2d4b09
Merge branch 'main' into pyodide-0-23-0
JeffersGlass 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
Bump kmeans timeout time
- Loading branch information
commit 74edd00d737dab154a6adc0e02ab0369bd96d559
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.
Terrible. What kind of test requires a twelve second timeout? Scipy is the worst.
It'll be really nice once jspi is shipping in browsers and we don't have to preload scipy's 100+ .so files. In the meantime I think we should get rid of kmeans. If a single test doesn't test anything and requires a twelve second timeout it needs to be gotten rid of. If it does test something can anyone tell me what it tests and why it can't be done in a scipy free way?
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.
to be honest it feels like we test too much 3rd party in our integration when the goal should be "if 3rd party can be loaded, we're good" so I am actually curious myself why we take responsibility for all these foreign modules ... I understand the will to ensure stuff works, but having these as part of our testing pipeline seems a bit off to me.
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.
to clarify further ... I think these tests should run only before a release and not for every commit we make to this project ... that would save time (both ours + computation) and grant we can tackle foreign modules gotchas before a release, not daily ... imho that would be great to me, so that code-freeze on release is meant to tackle possible gotchas and fix these if needed.
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.
@hoodmane for what it's worth, that's a 120 second timeout 🤢
I'm more than happy to go an entirely different direction with testing and removing the examples from the testing workflow, or testing them only prior to release, or what have you. I was mostly looking for a straightforward path to get this lon 8000 g-standing PR merged and the project moved up to Pyodide 0.23/Python 3.11.
How would we feel about merging this as-is and following up with a separate PR/discussion about improving the examples testing process?
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.
that would work for me
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.
Rather than only before the release, I'd personally prefer to run them nightly or weekly or something like that. At least we are aware something broke sooner (and we don't have the overhead of testing examples before merging a PR)
In terms of testing too much 3rd party, I agree that we should test less and mainly focus on "if they load, we are good". There are some caveats though that I'd like for us to consider and add better support to specific 3rd party libraries. That's especially true for modules that are quite popular and don't just work with specifying them as dependencies. Take
panel
orbokeh
as examples for instance, I'd like for us to make the user experience better and "auto-patch" everything they need to "just work" when you specify them as packages (but that is a totally different discussion and I think it probably belongs to PyScript core and rather to higher level API or something like this)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.
that's a great idea too, as long as these are off our daily pipes around unrelated tasks