Skip to content
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.

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

feat: upper cap check #501

Merged
merged 1 commit into from
Oct 6, 2024
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
2 changes: 1 addition & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ repos:
- pytest
- repo-review>=0.10.6
- rich
- tomli
- tomli>=2.0.2
- types-PyYAML

- repo: https://github.com/rbubley/mirrors-prettier
Expand Down
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,7 @@ for family, grp in itertools.groupby(collected.checks.items(), key=lambda x: x[1
### PyProject
- [`PP002`](https://learn.scientific-python.org/development/guides/packaging-simple#PP002): Has a proper build-system table
- [`PP003`](https://learn.scientific-python.org/development/guides/packaging-classic#PP003): Does not list wheel as a build-dep
- [`PP004`](https://learn.scientific-python.org/development/guides/packaging-simple#PP004): Does not upper cap Python requires
- [`PP301`](https://learn.scientific-python.org/development/guides/pytest#PP301): Has pytest in pyproject
- [`PP302`](https://learn.scientific-python.org/development/guides/pytest#PP302): Sets a minimum pytest to at least 6
- [`PP303`](https://learn.scientific-python.org/development/guides/pytest#PP303): Sets the test paths
Expand Down
5 changes: 5 additions & 0 deletions docs/pages/guides/packaging_simple.md
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,11 @@ build-backend = "setuptools.build_meta"

{% include pyproject.md %}

For `requires-python`, you should specify the minimum you require, and you
should not put an upper cap on it {% rr PY004 %}, as this field is used to
back-solve for old package versions that pass this check, allowing you to safely
drop Python versions.

## Package structure

All packages _should_ have a `src` folder, with the package code residing inside
Expand Down
43 changes: 43 additions & 0 deletions src/sp_repo_review/checks/pyproject.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

from typing import Any

from .._compat.importlib.resources.abc import Traversable
from . import mk_url


Expand Down Expand Up @@ -50,6 +51,48 @@ def check(pyproject: dict[str, Any]) -> bool:
return False


class PP004(PyProject):
"Does not upper cap Python requires"

url = mk_url("packaging-simple")

@staticmethod
def check(pyproject: dict[str, Any], package: Traversable) -> bool | None:
"""
You should never upper cap your Python requirement. This is rarely correct, and
tools like pip do not handle this properly even if it is correct. This field is used
to back-solve. If you want to specify versions you've tested on, use classifiers. If
you want to add a custom error message, add a build-type and/or runtime assert.
"""

match pyproject:
case {"project": {"requires-python": requires}}:
return "~=" not in requires and "<" not in requires
case {
"tool": {
"poetry": {
"dependencies": {
"python": str(requires) | {"version": str(requires)}
}
}
}
}:
return (
"^" not in requires and "~=" not in requires and "<" not in requires
)

setup_cfg = package / "setup.cfg"
if setup_cfg.is_file():
import configparser

config = configparser.ConfigParser()
config.read_string(setup_cfg.read_text(encoding="utf-8"))
if requires := config.get("options", "python_requires"):
return "~=" not in requires and "<" not in requires

return None


class PP301(PyProject):
"Has pytest in pyproject"

Expand Down
3 changes: 1 addition & 2 deletions src/sp_repo_review/checks/ruff.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,7 @@ def ruff(pyproject: dict[str, Any], root: Traversable) -> dict[str, Any] | None:
for path in paths:
if path.is_file():
with path.open("rb") as f:
# Type ignore fixed in https://github.com/hukkin/tomli/pull/215
contents = tomllib.load(f) # type: ignore[arg-type]
contents = tomllib.load(f)
if contents.get("extend", "") == "pyproject.toml":
extend = pyproject.get("tool", {}).get("ruff", {})
return merge(extend, contents)
Expand Down
67 changes: 67 additions & 0 deletions tests/test_pyproject.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
import inspect
from pathlib import Path

from repo_review.testing import compute_check, toml_loads


Expand Down Expand Up @@ -46,6 +49,70 @@ def test_PP003_has_wheel():
assert not compute_check("PP003", pyproject=toml).result


def test_PP004_no_cap_pyproject(tmp_path: Path):
toml = toml_loads("""
[project]
requires-python = ">=3.10"
""")

assert compute_check("PP004", pyproject=toml, package=tmp_path).result


def test_PP004_cap_pyproject(tmp_path: Path):
toml = toml_loads("""
[project]
requires-python = ">=3.10, <4"
""")

assert compute_check("PP004", pyproject=toml, package=tmp_path).result is False


def test_PP004_cap_tilde_pyproject(tmp_path: Path):
toml = toml_loads("""
[project]
requires-python = "~=3.10"
""")

assert compute_check("PP004", pyproject=toml, package=tmp_path).result is False


def test_PP004_cap_caret_pyproject(tmp_path: Path):
toml = toml_loads("""
[tool.poetry.dependencies]
python = "^3.10"
""")

assert compute_check("PP004", pyproject=toml, package=tmp_path).result is False


def test_PP004_setup_cfg_no_cap(tmp_path: Path):
(tmp_path / "setup.cfg").write_text(
inspect.cleandoc("""
[options]
python_requires = >=3.10
"""),
encoding="utf-8",
)

assert compute_check("PP004", pyproject={}, package=tmp_path).result


def test_PP004_setup_cfg_cap(tmp_path: Path):
(tmp_path / "setup.cfg").write_text(
inspect.cleandoc("""
[options]
python_requires = >=3.10,<4
"""),
encoding="utf-8",
)

assert compute_check("PP004", pyproject={}, package=tmp_path).result is False


def test_PP004_not_present(tmp_path: Path):
assert compute_check("PP004", pyproject={}, package=tmp_path).result is None


def test_PP302_okay_intstr():
toml = toml_loads("""
[tool.pytest.ini_options]
Expand Down