Skip to content

feat: added python/pip version check to make install #165

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

feat: added python/pip version check to make install #165

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8487904
core: added comments about PyPI actions
mike9ant Mar 19, 2025
31ede0a
Merge branch 'main' of github.com:deeporiginbio/deeporigin-client int…
mike9ant Mar 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,12 @@ jupyter:

# install in a virtual env with all extras
install:
@echo "Checking python version, venv and pip availability..."
@python3 -c 'import sys; exit(0 if sys.version_info >= (3,10) else 1)' \
|| (echo "You need Python 3.10 or higher. Please install it, e.g.: \n sudo apt install python3.10 python3.10-venv python3.10-dev" && exit 1)
@(python3 -m venv --help >/dev/null 2>&1 && python3 -m pip --version >/dev/null 2>&1) \
|| (echo "Python venv and/or pip are not available. Please install them, e.g.: \n sudo apt install python3.10-venv python3.10-dev python3-pip" && exit 1)

Comment on lines +46 to +51
Copy link
Collaborator

Choose a reason for hiding this comment

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

this won't work on macOS

Copy link
Collaborator

Choose a reason for hiding this comment

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

in general this is still a suboptimal solution, because we need specific things installed on the host, and we can't really control them. the way forward is to use something like uv to manage all the deps we need internally

Copy link
Contributor Author

Choose a reason for hiding this comment

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

by saying it won't work because the commands a bad, i.e. you have to use 'brew' instead? But the makefile will actually run, correct? if so maybe we check in and update later

Copy link
Contributor Author

Choose a reason for hiding this comment

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

or you saying version check may fail incorrectly, just causing install to fail where it wouldn't have?

Copy link
Collaborator

Choose a reason for hiding this comment

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

this line:

	@python3 -c 'import sys; exit(0 if sys.version_info >= (3,10) else 1)' \

is problematic, for example, on my computer:

❯ python3 --version
Python 3.9.6

@echo "Installing deeporigin in editable mode in a venv..."
@python3 -m venv venv
@source $(CURDIR)/venv/bin/activate && \
Expand Down