fix: determine package version even in deactivated venv

[jenstroeger]

Thank you @behnazh for noticing the regression with v2.4.1, introduced by PR #346. The problem is (still and again 🤦🏻‍♂️) that we don’t set up a venv in a Github runner and therefore we shouldn’t rely on the VIRTUAL_ENV environment variable.

This change expands the Python command so that the command always returns a package version string (which may be "unknown") instead of failing the import package.

I think it’s safe to expect "unknown" for make venv (or its Action equivalent)

python-package-template/.github/workflows/build.yaml

Lines 70 to 71 in 7bf474b

	- name: Create empty virtual environment for Actions
	run: mkdir .venv

and make setup

python-package-template/.github/workflows/build.yaml

Lines 72 to 73 in 7bf474b

	- name: Install dependencies
	run: make setup

because the package hasn’t been installed yet, and therefore its version can’t be determined.

However, after make setup the package’s version can be determined safely—with or without virtual environment. For example, the artifacts attached to workflow run 3488321082 indeed contain the correct package version number again:

[jenstroeger]

@behnazh after our conversation today we could add after these two lines

python-package-template/Makefile

Lines 6 to 8 in 66d2e0d

	# Set the package's name and version for use throughout the Makefile.
	PACKAGE_NAME := package
	PACKAGE_VERSION := $(shell python -c $$'try: import $(PACKAGE_NAME); print($(PACKAGE_NAME).__version__);\nexcept: print("unknown");')

the following

ifeq ($(PACKAGE_VERSION),unknown)
  $(warning Unable to determine package version, proceeding anyway)
endif

That way, a user shouldn’t be surprised anymore if "unknown" sneaks into file names.

[behnazh]

@behnazh after our conversation today we could add after these two lines

python-package-template/Makefile

Lines 6 to 8 in 66d2e0d

	# Set the package's name and version for use throughout the Makefile.
	PACKAGE_NAME := package
	PACKAGE_VERSION := $(shell python -c $$'try: import $(PACKAGE_NAME); print($(PACKAGE_NAME).__version__);\nexcept: print("unknown");')

the following

ifeq ($(PACKAGE_VERSION),unknown)
  $(warning Unable to determine package version, proceeding anyway)
endif

That way, a user shouldn’t be surprised anymore if "unknown" sneaks into file names.

Shouldn't we exit with a failure when package version is "unknown"?

[jenstroeger]

Shouldn't we exit with a failure when package version is "unknown"?

Thing is that "unknown" is valid when running make venv and make setup. We could add more checks to exclude these two goals, or issue a warning instead of error & exit.

[behnazh]

We could add more checks to exclude these two goals, or issue a warning instead of error & exit.

No, I think the code as it is captures the issues and we don't need more warnings.