Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
47 changes: 46 additions & 1 deletion rsconnect/pyproject.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import pathlib
import typing
import configparser

try:
import tomllib
Expand All @@ -20,18 +21,38 @@ def lookup_metadata_file(directory: typing.Union[str, pathlib.Path]) -> typing.L

The returned value is either a list of tuples [(filename, path)] or
an empty list [] if no metadata file was found.

The metadata files are returned in the priority they should be processed
to determine the python version requirements.
"""
directory = pathlib.Path(directory)

def _generate():
for filename in ("pyproject.toml", "setup.cfg", ".python-version"):
for filename in (".python-version", "pyproject.toml", "setup.cfg"):
path = directory / filename
if path.is_file():
yield (filename, path)

return list(_generate())


def get_python_version_requirement_parser(
metadata_file: pathlib.Path,
) -> typing.Optional[typing.Callable[[pathlib.Path], typing.Optional[str]]]:
"""Given the metadata file, return the appropriate parser function.

The returned function takes a pathlib.Path and returns the parsed value.
"""
if metadata_file.name == "pyproject.toml":
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm always wondering if we should be comparing filenames with a case-insensitive comparison. When I looked, google's AI provided this, so I think this might be a good improvement here:

The pyproject.toml filename is generally treated as case-insensitive by most Python tools and systems. While the file system itself might be case-sensitive, tools that interact with pyproject.toml, such as pip, poetry, and build, typically perform case-insensitive matching when searching for the file.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Given

https://github.com/pypa/pip/blob/main/src/pip/_internal/req/req_install.py#L512-L514

and

https://github.com/pypa/pip/blob/main/src/pip/_internal/pyproject.py#L26-L27

I think it's fairly safe to assume that the filename is expected to be lowercase for pip too.

It's true that the PEP doesn't explicitly say that the filename must be case sensitive, but in every single place it explicitly write pyproject.toml lowercase, which I think it's safe to expect it means the filename is expected to always be lowercase and thus work in both case sensitive and case insensitive file systems.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok, thanks for considering my suggestion.

return parse_pyproject_python_requires
elif metadata_file.name == "setup.cfg":
return parse_setupcfg_python_requires
elif metadata_file.name == ".python-version":
return parse_pyversion_python_requires
else:
return None
Copy link
Collaborator Author

@amol- amol- Mar 13, 2025

Choose a reason for hiding this comment

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

This could be done with a lookup table, but given the options aren't many I preferred to be explicit and avoid dynamic lookups which are harder to follow when debugging.



def parse_pyproject_python_requires(pyproject_file: pathlib.Path) -> typing.Optional[str]:
"""Parse the project.requires-python field from a pyproject.toml file.

Expand All @@ -43,3 +64,27 @@ def parse_pyproject_python_requires(pyproject_file: pathlib.Path) -> typing.Opti
pyproject = tomllib.loads(content)

return pyproject.get("project", {}).get("requires-python", None)


def parse_setupcfg_python_requires(setupcfg_file: pathlib.Path) -> typing.Optional[str]:
"""Parse the options.python_requires field from a setup.cfg file.

Assumes that the setup.cfg file exists, is accessible and well formatted.

Returns None if the field is not found.
"""
config = configparser.ConfigParser()
config.read(setupcfg_file)

return config.get("options", "python_requires", fallback=None)


def parse_pyversion_python_requires(pyversion_file: pathlib.Path) -> typing.Optional[str]:
"""Parse the python version from a .python-version file.

Assumes that the .python-version file exists, is accessible and well formatted.

Returns None if the field is not found.
"""
content = pyversion_file.read_text()
return content.strip()
68 changes: 63 additions & 5 deletions tests/test_pyproject.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,13 @@
import os
import pathlib

from rsconnect.pyproject import lookup_metadata_file, parse_pyproject_python_requires
from rsconnect.pyproject import (
lookup_metadata_file,
parse_pyproject_python_requires,
parse_setupcfg_python_requires,
parse_pyversion_python_requires,
get_python_version_requirement_parser,
)

import pytest

Expand All @@ -11,7 +17,7 @@
# Most of this tests, verify against three fixture projects that are located in PROJECTS_DIRECTORY
# - using_pyproject: contains a pyproject.toml file with a project.requires-python field
# - using_setupcfg: contains a setup.cfg file with a options.python_requires field
# - using_pyversion: contains a .python-version file and a pyproject.toml file without any version constraint.
# - using_pyversion: contains a .python-version file and pyproject.toml, setup.cfg without any version constraint.
# - allofthem: contains all metadata files all with different version constraints.


Expand All @@ -23,11 +29,12 @@
(
os.path.join(PROJECTS_DIRECTORY, "using_pyversion"),
(
"pyproject.toml",
".python-version",
"pyproject.toml",
"setup.cfg",
),
),
(os.path.join(PROJECTS_DIRECTORY, "allofthem"), ("pyproject.toml", "setup.cfg", ".python-version")),
(os.path.join(PROJECTS_DIRECTORY, "allofthem"), (".python-version", "pyproject.toml", "setup.cfg")),
],
ids=["pyproject.toml", "setup.cfg", ".python-version", "allofthem"],
)
Expand All @@ -37,6 +44,23 @@ def test_python_project_metadata_detect(project_dir, expected):
assert lookup_metadata_file(project_dir) == expectation


@pytest.mark.parametrize(
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe consider a comment describing this test?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There is one at line 58, do think it's not sufficiently informative?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, sorry, I caught myself seeing that for a different test and suppressed my comment. This is fine.

"filename, expected_parser",
[
("pyproject.toml", parse_pyproject_python_requires),
("setup.cfg", parse_setupcfg_python_requires),
(".python-version", parse_pyversion_python_requires),
("invalid.txt", None),
],
ids=["pyproject.toml", "setup.cfg", ".python-version", "invalid"],
)
def test_get_python_version_requirement_parser(filename, expected_parser):
"""Test that given a metadata file name, the correct parser is returned."""
metadata_file = pathlib.Path(PROJECTS_DIRECTORY) / filename
parser = get_python_version_requirement_parser(metadata_file)
assert parser == expected_parser


@pytest.mark.parametrize(
"project_dir",
[
Expand All @@ -59,9 +83,43 @@ def test_python_project_metadata_missing(project_dir):
ids=["option-exists", "option-missing"],
)
def test_pyprojecttoml_python_requires(project_dir, expected):
"""Test that the python_requires field is correctly parsed from pyproject.toml.
"""Test that the requires-python field is correctly parsed from pyproject.toml.

Both when the option exists or when it missing in the pyproject.toml file.
"""
pyproject_file = pathlib.Path(project_dir) / "pyproject.toml"
assert parse_pyproject_python_requires(pyproject_file) == expected


@pytest.mark.parametrize(
"project_dir, expected",
[
(os.path.join(PROJECTS_DIRECTORY, "using_setupcfg"), ">=3.8"),
(os.path.join(PROJECTS_DIRECTORY, "using_pyversion"), None),
],
ids=["option-exists", "option-missing"],
)
def test_setupcfg_python_requires(tmp_path, project_dir, expected):
"""Test that the python_requires field is correctly parsed from setup.cfg.

Both when the option exists or when it missing in the file.
"""
setupcfg_file = pathlib.Path(project_dir) / "setup.cfg"
assert parse_setupcfg_python_requires(setupcfg_file) == expected


@pytest.mark.parametrize(
"project_dir, expected",
[
(os.path.join(PROJECTS_DIRECTORY, "using_pyversion"), ">=3.8, <3.12"),
],
ids=["option-exists"],
)
def test_pyversion_python_requires(tmp_path, project_dir, expected):
"""Test that the python version is correctly parsed from .python-version.

We do not test the case where the option is missing, as an empty .python-version file
is not a valid case for a python project.
"""
versionfile = pathlib.Path(project_dir) / ".python-version"
assert parse_pyversion_python_requires(versionfile) == expected
4 changes: 4 additions & 0 deletions tests/testdata/python-project/using_pyversion/setup.cfg
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
[metadata]
name = python-project
version = 0.1.0
description = Add your description here
Loading