-
Notifications
You must be signed in to change notification settings - Fork 34
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
(ci) fix notebook environments in ci #907
Conversation
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.
Tests are green and the logic seems fine. 😊
Added just a couple of comments, they don't need to be done now, just a suggestion fro the future.
notebook=$1 | ||
|
||
# Now you can use the variable $notebook in your script | ||
echo "The path to the notebook is \"$notebook\"" |
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.
Perhaps some (most?) of those echo should use the debug output option/prefix, so that we only show what is necessary on a normal run?
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.
Yes, didn't know about this. Thx!
Will fix the next times these files are touched!
echo "Running replace_pip_install_notebooks.sh" | ||
sh .github/scripts/replace_pip_install_notebooks.sh ${{ matrix.notebook }} |
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.
The output of this could be wrapped in a log group. 😉
I confirm that this contribution is made under the terms of the license found in the root directory of this repository's source tree and that I have the authority necessary to make this contribution on behalf of its copyright owner.
This PR solves #905 while still allowing us to keep using
!pip3 install package1 package2
in the notebooks, so they they can be run directly from Colab.
To achieve this, I added a script that:
pyvespa
andvespacli
as they are already installed.additional_requirements.txt
Also, moved to using uv for package management for speed and ability to lock versions, in preparation for #824 . (For this workflow only for now, others to follow)