-
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
GitHub actions MacOS build fix #324
Conversation
This should reduce redundant code and ensure the install process is consistant
as suggested [here](DeepLabCut/DeepLabCut#2342)
This reduces code duplication and ensures that github actions reflect build process
This reverts commit db0e868.
This reverts commit 45e1927.
This reverts commit 2d6c43c.
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.
Nice standardization changes to our builds! I added a few updates that could help further but nonblocking.
@@ -36,13 +36,10 @@ jobs: | |||
sudo apt-get install libsndfile* | |||
sudo apt-get install xvfb | |||
python -m pip install --upgrade pip | |||
pip install kenlm==0.1 --global-option="--max_order=12" | |||
pip install psychopy==2023.2.1 --no-deps | |||
pip install attrdict3 |
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.
We could potentially add this in another install script and have the pip upgrade command as part of dev install
@@ -119,24 +113,21 @@ jobs: | |||
python-version: ${{ matrix.python-version }} | |||
- name: update pip & install custom dependencies | |||
run: | | |||
sh scripts/shell/m2chip_install.sh | |||
brew install labstreaminglayer/tap/lsl |
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.
Is this already part of the dev script?
tqdm==4.62.2 | ||
tables==3.7.0 |
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.
Was this missing from all builds or just Mac?
Overview
Fixes issue where macOS builds on the GitHub actions would fail, and refactors GitHub actions to use makefile.
Ticket
Pivotal ticket
Contributions
Test
Documentation
Updated comment in
m2chip_install.sh
to indicate it has been tested on M3. other than that, the changes don't require additional documentationChangelog