chore(python): default local dev environment to 3.7 for api/shared-data

[mcous]

Overview

#9497 removed the block [requires] python_version = "3.7" from api/Pipfile and shared-data/python/Pipfile so that pipenv would allow CI to install the dev environments on other versions of Python so we could test against Python 3.10 in CI.

(Unlike Poetry or other tools, Pipenv only allows to to specify an exact Python version requirement or nothing at all, which is super cool everyone.)

However, when running make setup locally for these projects, with the removal of that requirement, pipenv will select whatever version of Python that it wants to create the virtual environments of these projects. If/when pipenv selects something other than 3.7, you'll start getting all sorts of lint and typecheck failures.

Changelog

This PR updates our Makefiles to recognize an OT_VIRTUALENV_VERSION environment variable which will default to Python 3.7, and feeds into the --python option of pipenv. This means:

• When run locally, make setup will continue to select Python 3.7
• When run in CI, the Python Setup action can take a python-version input and feed it into this variable, maintaining Python 3.10 CI for api and shared-data

Review requests

• make teardown-py && make setup-py does what you expect, leaving you with a collection of Python 3.7 virtual envs
• API and shared-data CI builds continue to run on both 3.7 and 3.10

Risk assessment

Very low, local dev env + CI stuff, does not affect production Python evironment

[codecov]

Codecov Report

Merging #9708 (d81cdc7) into edge (32a5ed0) will decrease coverage by 0.00%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             edge    #9708      +/-   ##
==========================================
- Coverage   75.45%   75.45%   -0.01%     
==========================================
  Files        1937     1937              
  Lines       51039    51039              
  Branches     4855     4855              
==========================================
- Hits        38512    38509       -3     
- Misses      11610    11613       +3     
  Partials      917      917              

Flag	Coverage Δ
api	85.08% <ø> (ø)
app	71.76% <ø> (ø)
hardware	75.09% <ø> (-0.17%)	⬇️
protocol-designer	44.16% <ø> (ø)
robot-server	93.07% <ø> (+0.02%)	⬆️
shared-data	84.81% <ø> (ø)
step-generation	90.33% <ø> (ø)
update-server	73.19% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...are/hardware_control/motion_planning/move_utils.py	90.21% <0.00%> (-2.18%)	⬇️
robot-server/robot_server/hardware.py	76.69% <0.00%> (+0.97%)	⬆️

[SyntaxColoring]

CI is failing on this PR and I don't know why—GitHub isn't showing me logs, for some reason.

Other than that, this makes sense to me, in principle.

It's unfortunate that our build system is getting so complicated. I can't think of any better ways to solve this, though. :\

[SyntaxColoring]

I think the comment above might be confusing, given this change.

My understanding is that OT_PYTHON_VERSION only matters when pyenv or similar is available. It influences which of the many installed Pythons are chosen by Pipenv. So it's kind of the opposite of OT_PYTHON?

Maybe OT_PYTHON and OT_PYTHON_VERSION shouldn't be grouped together, or maybe they should have more specific names.

[SyntaxColoring]

Aside:

I haven't looked at api yet, but shared-data's makefile has a pretty weird knot. I don't think there's any action needed in this PR, but here's some investigation.

It has these dynamically-built helper variables:

opentrons/shared-data/python/Makefile

Lines 31 to 32 in 32a5ed0

	wheel_file = $(BUILD_DIR)/$(call python_get_wheelname,shared-data,opentrons_shared_data,$(BUILD_NUMBER),../../scripts/python_build_utils.py)
	sdist_file = $(BUILD_DIR)/$(call python_get_sdistname,shared-data,opentrons_shared_data,,../../scripts/python_build_utils.py)

If you follow call python_get_wheelname and call python_get_wheelname into scripts/python.mk, you can see that they eventually do a $(pipenv) run python. And if there isn't already a virtual envirnment, $(pipenv) run python will automatically create one—without taking into account the new OT_VIRTUALENV_VERSION thing you've added.

So, merely evaluating this expression will preempt the correct Pipenv command to set up the virtual environment, creating an incorrect one in its place.

You might think this is okay because the definitions use = instead of :=, so evaluation will happen at usage time instead of definition time. And this is true. But these variables show up at the top level, as dependencies and target names:

opentrons/shared-data/python/Makefile

Lines 71 to 72 in 32a5ed0

	.PHONY: wheel
	wheel: $(wheel_file)

opentrons/shared-data/python/Makefile

Lines 79 to 82 in 32a5ed0

	$(wheel_file): setup.py $(py_sources) $(json_sources)
	$(SHX) mkdir -p build
	$(python) setup.py $(wheel_opts) bdist_wheel
	$(SHX) ls $(BUILD_DIR)

So, apparently, even reading this Makefile will cause a $(pipenv run python) to create a wrong virtual environment before make setup has a chance to create the correct one.

Fortunately, it looks like a subsequent make setup will delete the incorrect virtual environment and replace it with one built with OT_VIRTUALENV_VERSION. So in the end, I think it just does redundant work and causes confusing terminal output.