Skip to content

Make create-env-files.sh respect min python version supported #1446

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

Closed
mhucka wants to merge 1 commit into from
Closed

Make create-env-files.sh respect min python version supported #1446

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
50 changes: 35 additions & 15 deletions dev_tools/requirements/create-env-files.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,36 +16,56 @@
set -o errexit
set -o nounset

declare -r usage="Usage: ${0} [-h] [UV_OPTIONS]
# Go to the top of the local TFQ git tree. Do it early in case this fails.
script_dir=$(CDPATH="" cd -- "$(dirname -- "${0}")" && pwd -P)
repo_dir=$(git -C "${script_dir}" rev-parse --show-toplevel 2>/dev/null)

@pavoljuhas pavoljuhas Aug 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should keep the stderr message so user gets a clue what is going on if the script exits here on git-rev-parse failure.

Suggested change
repo_dir=$(git -C "${script_dir}" rev-parse --show-toplevel 2>/dev/null)
repo_dir=$(git -C "${script_dir}" rev-parse --show-toplevel)

cd "${repo_dir}"
Comment on lines +20 to +22

@gemini-code-assist gemini-code-assist Bot Aug 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

If the script is run outside of a git repository (for example, from a source archive/tarball) or if git is not installed, repo_dir will be empty. Because set -o errexit is enabled, the subsequent cd "${repo_dir}" will fail with a cryptic error and terminate the script immediately. We should check if repo_dir is empty and provide a clear, helpful error message.

Suggested change
script_dir=$(CDPATH="" cd -- "$(dirname -- "${0}")" && pwd -P)
repo_dir=$(git -C "${script_dir}" rev-parse --show-toplevel 2>/dev/null)
cd "${repo_dir}"
script_dir=$(CDPATH="" cd -- "$(dirname -- "${0}")" && pwd -P)
repo_dir=$(git -C "${script_dir}" rev-parse --show-toplevel 2>/dev/null)
if [[ -z "${repo_dir}" ]]; then
echo "Error: Failed to determine repository root. Are you in a git repository?" >&2
exit 1
fi
cd "${repo_dir}"


default_min_python=$(python3 -c 'import re

@pavoljuhas pavoljuhas Aug 18, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is way too complicated and as is it extracts the black formatter setting which may be unrelated to the actual supported version. A better way would be to grep setup.py for python_requires, but I feel even that would be more fragile and more laborious to maintain in a long term than to just set

default_min_python=3.10

and bump it up when the minimum required version goes up.

Add: not needed at all per #1446 (comment)

@mhucka mhucka Aug 18, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Setup.py is going away in another PR, which is why this doesn't look in setup.py

s = open("pyproject.toml").read()
m = re.search(r"requires-python.*?>=?(\d+\.\d+)", s)
if not m:
m = re.search(r"target-version.*?py(\d)(\d+)", s)
print(m.group(1) if "requires-python" in m.re.pattern else m.expand(r"\1.\2"))
' 2>/dev/null || echo "3.10")
Comment on lines +24 to +30

@gemini-code-assist gemini-code-assist Bot Aug 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

If pyproject.toml exists but contains neither requires-python nor target-version, the python script will raise an unhandled AttributeError when trying to access m.re or m.expand on a None object. While the shell script catches this with || echo "3.10", relying on silent python crashes is fragile and makes debugging harder. We should handle the None case and potential exceptions gracefully within the Python snippet.

Suggested change
default_min_python=$(python3 -c 'import re
s = open("pyproject.toml").read()
m = re.search(r"requires-python.*?>=?(\d+\.\d+)", s)
if not m:
m = re.search(r"target-version.*?py(\d)(\d+)", s)
print(m.group(1) if "requires-python" in m.re.pattern else m.expand(r"\1.\2"))
' 2>/dev/null || echo "3.10")
default_min_python=$(python3 -c 'import re
try:
s = open("pyproject.toml").read()
m = re.search(r"requires-python.*?>=?(\d+\.\d+)", s)
if m:
print(m.group(1))
else:
m = re.search(r"target-version.*?py(\d)(\d+)", s)
print(m.expand(r"\1.\2") if m else "3.10")
except Exception:
print("3.10")
' 2>/dev/null)

@pavoljuhas pavoljuhas Aug 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Slop and Bloat Gemini, Yuck!


declare -r usage="Usage: ${0} [-h] [-p X.Y] [UV_OPTIONS]
Generate environment files for OpenFermion development using uv's
'universal' option, making the result compatible with multiple
Python versions. The output is written to subdirectories under
dev_tools/requirements/.

Options:
-h Show this help message and exit
-h Show this help message and exit

@pavoljuhas pavoljuhas Aug 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
-h Show this help message and exit
-h, --help Show this help message and exit

-p X.Y Minimum Python version to support (default: ${default_min_python})

All other options on the command line will be passed directly to
'uv pip compile'. Run 'uv pip compile --help' to learn about the
options available."

min_python=("--python-version" "${default_min_python}")

while [[ $# -gt 0 ]]; do
case "${1}" in
-h|--help) echo "${usage}"; exit 0 ;;
-p|--min-python|--python-version)

@pavoljuhas pavoljuhas Aug 18, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is shadowing the uv-pip -p, --python option which would be ignored if specified first, but processed if placed as a later argument. In addition, user would not be able to pass --python-version=X.Y.Z because uv does not accept that option multiple times.

I recommend to add the python-version setting to pyproject.toml instead and drop the custom -p, --min-python option here altogether:

diff --git a/pyproject.toml b/pyproject.toml
index 72ba61d..b8de85d 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -73,4 +73,5 @@ follow_untyped_imports = true
 [tool.uv.pip]
 universal = true
 generate-hashes = true
 custom-compile-command = "dev_tools/requirements/create-env-files.sh"
+python-version = "3.10"

if [[ $# -lt 2 || -z "${2:-}" ]]; then
echo "Error: option '${1}' requires an argument" >&2
echo "${usage}" >&2
exit 1
fi
min_python=("--python-version" "${2}")
shift 2
;;
*) break ;;
esac
done
Comment on lines 48 to 62

@gemini-code-assist gemini-code-assist Bot Aug 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

The current argument parsing loop terminates on the first unrecognized option (using *) break ;;). If a user passes any other options (such as --upgrade or --no-cache) before -p or --min-python, the script will stop parsing arguments, leaving -p unparsed and passing it directly to uv pip compile, which will fail because uv does not recognize -p.

We can fix this by collecting unrecognized options into an array and resetting the positional parameters ($@) at the end of the loop.

Suggested change
while [[ $# -gt 0 ]]; do
case "${1}" in
-h|--help) echo "${usage}"; exit 0 ;;
-p|--min-python|--python-version)
if [[ $# -lt 2 || -z "${2:-}" ]]; then
echo "Error: option '${1}' requires an argument" >&2
echo "${usage}" >&2
exit 1
fi
min_python=("--python-version" "${2}")
shift 2
;;
*) break ;;
esac
done
uv_options=()
while [[ $# -gt 0 ]]; do
case "${1}" in
-h|--help) echo "${usage}"; exit 0 ;;
-p|--min-python|--python-version)
if [[ $# -lt 2 || -z "${2:-}" ]]; then
echo "Error: option '${1}' requires an argument" >&2
echo "${usage}" >&2
exit 1
fi
min_python=("--python-version" "${2}")
shift 2
;;
*)
uv_options+=("${1}")
shift
;;
esac
done
set -- "${uv_options[@]}"


# Go to the top of the local TFQ git tree. Do it early in case this fails.
script_dir=$(CDPATH="" cd -- "$(dirname -- "${0}")" && pwd -P)
repo_dir=$(git -C "${script_dir}" rev-parse --show-toplevel 2>/dev/null)
cd "${repo_dir}"

mkdir -p dev_tools/requirements/envs dev_tools/requirements/max_compat

# ~~~~ Generate basic requirements files ~~~~

uv pip compile "$@" \
uv pip compile "${min_python[@]}" "$@" \

@pavoljuhas pavoljuhas Aug 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This and below can be reverted after setting python-version in pyproject.toml.

-o dev_tools/requirements/envs/dev.env.txt \
dev_tools/requirements/deps/format.txt \
dev_tools/requirements/deps/mypy.txt \
Expand All @@ -56,45 +76,45 @@ uv pip compile "$@" \
dev_tools/requirements/deps/runtime.txt \
dev_tools/requirements/deps/shellcheck.txt

uv pip compile "$@" \
uv pip compile "${min_python[@]}" "$@" \
-o dev_tools/requirements/envs/format.env.txt \
-c dev_tools/requirements/envs/dev.env.txt \
dev_tools/requirements/deps/format.txt \
dev_tools/requirements/deps/runtime.txt

uv pip compile "$@" \
uv pip compile "${min_python[@]}" "$@" \
-o dev_tools/requirements/envs/pylint.env.txt \
-c dev_tools/requirements/envs/dev.env.txt \
dev_tools/requirements/deps/pylint.txt \
dev_tools/requirements/deps/runtime.txt

uv pip compile "$@" \
uv pip compile "${min_python[@]}" "$@" \
-o dev_tools/requirements/envs/pytest.env.txt \
-c dev_tools/requirements/envs/dev.env.txt \
dev_tools/requirements/deps/pytest.txt \
dev_tools/requirements/deps/runtime.txt

uv pip compile "$@" \
uv pip compile "${min_python[@]}" "$@" \
-o dev_tools/requirements/envs/pytest-extra.env.txt \
-c dev_tools/requirements/envs/dev.env.txt \
dev_tools/requirements/deps/pytest.txt \
dev_tools/requirements/deps/resource_estimates_runtime.txt \
dev_tools/requirements/deps/runtime.txt

uv pip compile "$@" \
uv pip compile "${min_python[@]}" "$@" \
-o dev_tools/requirements/envs/mypy.env.txt \
-c dev_tools/requirements/envs/dev.env.txt \
dev_tools/requirements/deps/mypy.txt \
dev_tools/requirements/deps/runtime.txt

uv pip compile "$@" \
uv pip compile "${min_python[@]}" "$@" \
-o dev_tools/requirements/envs/shellcheck.env.txt \
-c dev_tools/requirements/envs/dev.env.txt \
dev_tools/requirements/deps/shellcheck.txt

# ~~~~ Generate max_compat files ~~~~

uv pip compile "$@" \
uv pip compile "${min_python[@]}" "$@" \
-o dev_tools/requirements/max_compat/pytest-max-compat.env.txt \
-c dev_tools/requirements/deps/oldest-versions.txt \
dev_tools/requirements/deps/pytest.txt \
Expand Down
Loading