feat: support repo-review

[henryiii]

Would you be interested in integrating with repo-review? I've drafted up an example of what would be required for it in this PR, it doesn't add any dependencies.

For more on repo-review, see the docs or this post. You can see a WebAssembly powered version at learn.scientific-python.org. I'm pretty sure from my testing that this will work fine with the WebAssembly version. :)

Here's how you can test it:

$ virtualenv .venv
$ . .venv/bin/activate
$ pip install -e . repo-review
$ repo-review .
Validate-PyProject:
Checks [build-system], [project], [tool.distutils], [tool.setuptools]

└── VPP001 Validate pyproject.toml ✅

$ repo-review gh:numpy/numpy@main
Processing gh:numpy/numpy@main:. from GitHub

Validate-PyProject:
Checks [build-system], [project], [tool.distutils], [tool.setuptools]

└── VPP001 Validate pyproject.toml ✅

You can add foo = "bar" to the local pyproject.toml and try again:

$ repo-review .
Validate-PyProject:
Checks [build-system], [project], [tool.distutils], [tool.setuptools]

└── VPP001 Validate pyproject.toml ❌
    Invalid pyproject.toml! Error: build-system must not contain {'foo'} properties

WDYT?

[abravalheri]

Thank you very much @henryiii, it does look very interesting and probably very good to add the integration.

I have a question, though:

• In the example we have Checks [build-system], [project], [tool.distutils], [tool.setuptools].
  Would it be possible to hide [tool.distutils] and only show it if [tool.distutils] is actually present in the given pyproject.toml?
  The reason why I am asking this is because I don't want to incentivise people to go around using [tool.distutils], since it was not fully discussed in the context of setuptools (it serves a purpose and I think we have to keep it there, but I am not fully convinced about the name of the section -- I kind of bikeshed myself on that)¹.

Footnotes

1. In the context of ini2toml and setuptools.config.pyprojecttoml I implemented that because the overall goal it to eventually replace setuptools parsing of setup.cfg with automatically translation to TOML see https://github.com/pypa/setuptools/issues/1688#issuecomment-471212342. But my impression is that [tool.distutils] is not in its final form yet. ↩

[henryiii]

I was wondering about that! Yes, it’s easily possible. All functions (fixtures, checks, family collection, and check collection) can request fixtures. So we can add it above. Will do later.

[henryiii]

Not sure what to do about coverage. It'd not happy for Python 3.6-3.9 since repo-review is 3.10+ only. Combined coverage would be fine, and 3.10+ is fine.

[abravalheri]

Thank you very much @henryiii, that looks very good.

Not sure what to do about coverage. It'd not happy for Python 3.6-3.9 since repo-review is 3.10+ only. Combined coverage would be fine, and 3.10+ is fine.

Coverage reports are always tricky aren't they? For me it feels always like a cat and mouse game...
There seems to be a couple of coverage plugins out there that can help with conditional coverage (e.g. https://github.com/wemake-services/coverage-conditional-plugin), but I have never used them. So I don't know... Do you think it would be a good idea to use something like that in this case? I am happy to just ignore the coverage drop btw, just wondering if this is an opportunity to explore these plugins (I can add them myself to the project so you are not trapped in an endless review cycle).

I think the PR is in a good shape and can be included in the next release of validate-pyproject.
Thank you again for the contribution.

I would like to discuss a few minor things that I was planning to do after the merge to see your feedback. These are changes that I can do myself if you are OK with them:

• Is it OK if I change type annotations a little bit to remove from __future__ import annotations? I know that Python 3.6 is long dead and that repo-review is Python3.10+, but I like to keep the "LTS" running a bit longer for as long as it does not cost me too much in terms of development/maintenance efforts 😝 (so it would be nice if there are no errors while importing the modules in Python 3.6...)
• Would it be OK if you if I condensate families and checks into a single file? I know that it is nice to keep files separated and clean, but because the implementation is so short, it looks a bit unfair compared to the other files in the repo (my bad for writing big files 😝).

[henryiii]

wondering if this is an opportunity to explore these plugins

Up to you. IMO, you have to combine coverage to make it useful, there will pretty much always be version dependent code. Otherwise, you could just run one coverage job (3.10 or 3.11), and just use that. Quite up to you, though.

Would it be OK if you if I condensate

Sure, I went ahead and did that.

Is it OK if I change type annotations a little bit to remove ... does not cost me too much in terms of development/maintenance efforts

Sure, I went ahead and did that. Though, I don't think it's actually helpful to continue to support old Python versions, it's negativity affecting this repo (no future import causing dependence on deprecated Dict and such, as well as that's there reason that PEP 621 isn't available, etc) and actually may increase the maintenance burden for others. For example, Cython 3 was released including Python 2.7 & 3.4+ (at least 2.6 was dropped!). This caused breakages with packages that have no interest in ever releasing a fix for these old versions (pyyaml, for example, is now broken on builds from source on 2.7), and new code isn't really being written that really needs Cython 3 on these old platforms. The goal is to keep them working, not add features (which breaks more than helps!) But do what you want. :)

[abravalheri]

Thank you very much @henryiii.

It looks very good. I think it is ready for merge (and then I will decide what to do with the coverage :P)