Skip to content

Update packaging guide and repo-review to match spec13 #438

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

Closed
wants to merge 1 commit into from
Closed
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 .github/workflows/reusable-rr-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ jobs:
python-version: ${{ matrix.python-version }}

- name: Install package
run: python -m pip install .[test,cli]
run: python -m pip install .[tests,cli]

- name: Test package
run: python -m pytest -ra
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ backports structure with a small typing example.
- An pylint nox target can be used to run pylint, which integrated GHA
annotations
- A ReadTheDocs-ready Sphinx docs folder and `[docs]` extra
- A test folder and pytest `[test]` extra
- A test folder and pytest `[tests]` extra
- A noxfile is included with a few common targets

Setuptools only:
Expand Down
2 changes: 1 addition & 1 deletion docs/_includes/pyproject.md
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ Here is an example of a simple extras:

```toml
[project.optional-dependencies]
test = [
tests = [
"pytest >=6.0",
]
mpl = [
Expand Down
2 changes: 1 addition & 1 deletion docs/pages/guides/gha_basic.md
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ tests:
allow-prereleases: true

- name: Install package
run: python -m pip install -e .[test]
run: python -m pip install -e .[tests]

- name: Test package
run: python -m pytest
Expand Down
6 changes: 3 additions & 3 deletions docs/pages/guides/gha_wheels.md
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,9 @@ test-command = "pytest {project}/tests"
build-verbosity = 1
```

The `test-extras` will cause the pip install to use `[test]`. The `test-command`
will use pytest to run your tests. You can also set the build verbosity (`-v` in
pip) if you want to.
The `test-extras` will cause the pip install to use `[tests]`. The
`test-command` will use pytest to run your tests. You can also set the build
verbosity (`-v` in pip) if you want to.

## Making an SDist

Expand Down
2 changes: 1 addition & 1 deletion docs/pages/guides/packaging_classic.md
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,7 @@ called `package`:

```ini
[options.extras_require]
test =
tests =
pytest >=6.0
mpl =
matplotlib >=2.0
Expand Down
4 changes: 2 additions & 2 deletions docs/pages/guides/tasks.md
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ def tests(session: nox.Session) -> None:
"""
Run the unit and regular tests.
"""
session.install(".[test]")
session.install(".[tests]")
session.run("pytest", *session.posargs)
```

Expand Down Expand Up @@ -212,7 +212,7 @@ def tests(session: nox.Session) -> None:
"""
Run the unit and regular tests.
"""
session.install(".[test]")
session.install(".[tests]")
session.run("pytest", *session.posargs)
```
<!-- prettier-ignore-end -->
Expand Down
2 changes: 1 addition & 1 deletion noxfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ def tests(session: nox.Session, backend: str, vcs: bool) -> None:
session.chdir(cookie)

name = f"cookie-{backend}"
session.install(".[test]")
session.install(".[tests]")
session.run("python", "-m", "pytest", "-ra")
version = session.run(
"python",
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ dependencies = [
cli = [
"repo-review[cli]",
]
test = [
tests = [
"pytest >=7",
"repo-review >=0.10.6",
]
Expand Down
15 changes: 14 additions & 1 deletion src/sp_repo_review/checks/general.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,23 @@ class PY004(General):

@staticmethod
def check(package: Traversable) -> bool:
"Projects must have documentation in a folder called docs (disable if not applicable)"
"Projects must have documentation in a folder called doc or docs (disable if not applicable)"
return len([p for p in package.iterdir() if "doc" in p.name]) > 0


class PY004b(General):
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think a check can end in a letter. I think this could be merged into the PY004 check? Though some projects have multiple docs-* folders, it's important not to error on those. Maybe just making sure /doc doesn't exist is an option?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it works :-)

├── PY003 Has a LICENSE* file ✅
├── PY004 Has docs folder ✅
├── PY004b Documentation folder should be `docs` not `doc` ❌
│   Projects must have documentation in a folder called docs not doc

One of my main concern was to not mark a project as "not having docs", and think of a plan to convey to projects that doc used to be ok.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh, interesting. I feel like I've encoded this somewhere. Maybe not.

"Documentation folder should be `docs` not `doc`"

requires = {"PY004"}

url = mk_url("packaging-simple")

@staticmethod
def check(package: Traversable) -> bool:
"Projects must have documentation in a folder called `docs` not `doc`"
return any(p.name == "docs" for p in package.iterdir())


class PY005(General):
"Has tests folder"

Expand Down
60 changes: 60 additions & 0 deletions src/sp_repo_review/checks/pyproject.py
Original file line number Diff line number Diff line change
Expand Up @@ -241,5 +241,65 @@ def check(pyproject: dict[str, Any]) -> bool:
return "filterwarnings" in options


class PP310(PyProject):
"Tests target is test not test (spec13)"
Copy link
Collaborator

@henryiii henryiii Jun 5, 2024

Choose a reason for hiding this comment

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

Suggested change
"Tests target is test not test (spec13)"
"Tests extra is `tests` not `test` (spec13)"

?


requires = {"PP301"}
url = mk_url("pytest")

@staticmethod
def check(pyproject: dict[str, Any]) -> bool | None:
"""

Tests target should be `tests` not `test`

```toml
[project.optional-dependencies]
tests = [
'pytest',
...
]
```
"""
if "tool" not in pyproject:
return None
if "project.optional-dependencies" not in pyproject["tool"]:
return None
optional_deps = pyproject["tool"]["project.optional-dependencies"]
if "tests" in optional_deps:
return True
return "test" not in optional_deps
Comment on lines +264 to +271
Copy link
Collaborator

@henryiii henryiii Jun 5, 2024

Choose a reason for hiding this comment

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

Suggested change
if "tool" not in pyproject:
return None
if "project.optional-dependencies" not in pyproject["tool"]:
return None
optional_deps = pyproject["tool"]["project.optional-dependencies"]
if "tests" in optional_deps:
return True
return "test" not in optional_deps
match pyproject:
case {"tool": "project" { "optional-dependencies": {"tests": _}}}:
return True
case {"tool": "project" { "optional-dependencies": {"test": _}}}:
return False
case _:
return True

Try pattern matching here. ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I need to practice more my pattern matching :-)

Copy link
Collaborator

Choose a reason for hiding this comment

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

SPEC 0 should make that easier now. :)



class PP311(PyProject):
"Tests target is `docs not` `doc` (spec13)"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
"Tests target is `docs not` `doc` (spec13)"
"Tests target is `docs` not `doc` (SPEC13)"


requires = {"PP301"}
url = mk_url("pytest")

@staticmethod
def check(pyproject: dict[str, Any]) -> bool | None:
"""

docs target should be `docs` not `doc`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
docs target should be `docs` not `doc`
docs extra should be `docs` not `doc`

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also, I am not interested in having a check that goes against the official Python packaging specifications, which state test and doc are reserved for this. This means tools (like Hatch) can assume special meanings for these extras. IMO, SPECs are only for standardizing things not already standardized across Python, and so we can't enforce a SPEC over a the official core specification.

https://packaging.python.org/en/latest/specifications/core-metadata/#provides-extra-multiple-use

However, given that [docs] is more popular, and I can't see any PEP source for that recommendation (other than PEPs stating it is the source of truth), I think we could see if it can be changed or removed, in which case such a check is fine.


```toml
[project.optional-dependencies]
docs = [
'sphinx',
...
]
```
"""
if "tool" not in pyproject:
return None
if "project.optional-dependencies" not in pyproject["tool"]:
return None
optional_deps = pyproject["tool"]["project.optional-dependencies"]
if "docs" in optional_deps:
return True
return "doc" not in optional_deps


def repo_review_checks() -> dict[str, PyProject]:
return {p.__name__: p() for p in PyProject.__subclasses__()}
2 changes: 1 addition & 1 deletion {{cookiecutter.project_name}}/noxfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ def tests(session: nox.Session) -> None:
"""
Run the unit and regular tests.
"""
session.install(".[test]")
session.install(".[tests]")
session.run("pytest", *session.posargs)


Expand Down
8 changes: 4 additions & 4 deletions {{cookiecutter.project_name}}/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ pytest = ">= 6"
pytest-cov = ">= 3"

[tool.poetry.extras]
test = ["pytest", "pytest-cov"]
tests = ["pytest", "pytest-cov"]
dev = ["pytest", "pytest-cov"]
docs = [
"furo",
Expand Down Expand Up @@ -195,7 +195,7 @@ dynamic = ["version"]
dependencies = []

[project.optional-dependencies]
test = [
tests = [
"pytest >=6",
"pytest-cov >=3",
]
Expand Down Expand Up @@ -259,7 +259,7 @@ build.hooks.vcs.version-file = "src/{{ cookiecutter.__project_slug }}/_version.p

[tool.hatch.envs.default]
features = ["test"]
scripts.test = "pytest {args}"
scripts.tests = "pytest {args}"


{%- elif cookiecutter.backend == "pdm" %}
Expand All @@ -281,7 +281,7 @@ path = "src/{{ cookiecutter.__project_slug }}/__init__.py"
{%- endif %}

[tool.pdm.dev-dependencies]
devtest = ["pytest", "pytest-cov"]
devtests = ["pytest", "pytest-cov"]

{%- endif %}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ tests:
- if: $CI_PIPELINE_SOURCE == "push"
script:
- python -V
- python -m pip install .[test]
- python -m pip install .[tests]
- python -m pytest -ra --cov={{ cookiecutter.project_name }}
parallel:
matrix:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,6 @@ docs =
sphinx>=7.0
sphinx-autodoc-typehints
sphinx-copybutton
test =
tests =
pytest>=6
pytest-cov>=3
Loading