feat: requires-python

[henryiii]

I think this finishes #528's changes that affect non-Universal builds.

This is using CIBW_PROJECT_REQUIRES_PYTHON, though it could be changed to CIBW_IGNORE_REQUIRES_PYTHON and just turn off this search instead.

---

Docs example in PR.

---

With a proper pyproject.toml or setup.cfg file, the minimal example for a small package with cibuildwheel could be just 13 lines (using #542)!

on: [push, pull_request]

jobs:
  build_wheels:
    strategy:
      matrix:
        os: [ubuntu-20.04, windows-2019, macOS-10.15]
    runs-on: ${{ matrix.os }}

    steps:
      - uses: actions/checkout@v2
        
      - uses: joerick/[email protected]

      - uses: actions/upload-artifact@v2
        with:
          path: ./wheelhouse/*.whl

(Most packages should still set up a matrix to take better advantage of the 10+ parallel jobs GHA gives, but still, very nice!)

Fixes #528
Includes the MyPy update to 0.800, closes #561.

[henryiii]

Wait, why can you rerun a single Azure job directly from the GH interface, but not Github Actions?

[Czaki]

Wait, why can you rerun a single Azure job directly from the GH interface, but not Github Actions?

Because Github Actions does not support reboot a single job. Yes, both are provided by a single company.

[joerick]

I'm gonna hold off on reviewing this until we have a consensus on the design discussion in #528.

[henryiii]

Oops, the name is "project", not "metadata" (though that was listed as the second most popular choice ;) ), I will update everything at once when we decide on #528. Also, from reading the docs, this might be treated as all or nothing by build backends; that is, if someone -only- specifies project.requires-python, then setuptools would be allowed or possibly required to require all other non-optional fields be specified, unless they are explicitly listed in project.dynamic. So having the workaround might be extra important for the next year or two. As far as I can tell, though, it might only be "name" that's also required. Feel free to check out PEP 621.

[henryiii]

I've updated this to my favorite version of the proposal so far:

• CIBW_PROJECT_REQUIRES_PYTHON overrides pyproject.toml, which overrides setup.cfg as specified by PEP 621. The description focuses on setting this for the project, mentining it can be given by an environment variable if required (with warning). The files are no longer even read if this is set.
• If unset, no change applied, all versions of Python build (assuming this will not change in 2.0 in this case). The direction for "nice" builds is to set this via one of the above methods.

We still need unit tests to verify that cibuildwheel picks up pyproject.toml/setup.cfg expressions correctly.

[henryiii]

Added unit tests and fixed a few naming issues (python_requires is under [options] in setup.cfg, not [metadata]).

[joerick]

I think this API is going to work. The patch version question might throw some spanners into the works, but it shouldn't be insurmountable. Linux is the interesting one, because we don't hardcode the versions, perhaps we can call python --version to get it.

[joerick]

Hmm, what about patch versions? Maybe a package specifies python_requires >=3.9.1 because they rely on a bug fix? Might not be common, but I think that's allowed syntax. Also, does packaging consider 3.9 to match >=3.9.0?

[YannickJadoul]

Maybe a package specifies python_requires >=3.9.1 because they rely on a bug fix?

That would never happen! What crappy project maintainers would test the Python release candidates so badly that they allow this to happen? cough cough

[henryiii]

3.9.0 == 3.9 is true, sadly 3.9.1 == 3.9 is false (I thought it was true at one point so I likely have a few bad version_requires lying around; you really do need to write !=3.4.*).

test the Python release candidates so badly

Never! I'm sure it would show up months before release...

[joerick]

I had a go a rewriting this in #528, I think it might be better to go with something like this-

Suggested change

	### `pyproject.toml:project.requires-python` / `setup.cfg:options.python_requires` / `CIBW_PROJECT_REQUIRES_PYTHON` {: #requires-python}
	> Limit the Python versions to build
	The provisionally accepted [PEP 621](https://www.python.org/dev/peps/pep-0621/)
	setting in `pyproject.toml` for `project.requires-python` is respected by
	cibuildwheel. If that value is not present, setup.cfg's
	`options.python_requires` will be used instead. You can override this if
	needed using an environment variable; be warned that a wheel built for a
	disallowed Python will not be installed by pip without a workaround, like
	`--ignore-requires-python`. You do not have to manually avoid versions that
	cibuildwheel does not support; `>=2.7` and `>=2.7, !=3.0.*, !=3.1.*, !=3.2.*,
	!=3.3.*, !=3.4.*,` are equivalent, as far as cibuildwheel is concerned.
	Default: All versions of Python cibuildwheel supports.
	Example `pyproject.toml`:
	```toml
	[project]
	requires-python = ">=3.6"
	```
	Remember to always set `build-system.requires` and `build-system.build-backend`
	if you add a `pyproject.toml` file.
	Platform-specific variants also available:<br/>
	`CIBW_PROJECT_REQUIRES_PYTHON_MACOS` | `CIBW_PROJECT_REQUIRES_PYTHON_WINDOWS` | `CIBW_PROJECT_REQUIRES_PYTHON_LINUX`
	### CIBW_PROJECT_REQUIRES_PYTHON
	> Manually set the Python compatibility of your project
	By default, cibuildwheel reads your package's Python compatibility from
	pyproject.toml or setup.cfg. If you need to override this behaviour for some
	reason, you can use this option.
	When setting this option, the syntax is the same as `project.requires-python`,
	using 'version specifiers' like `>=3.6`, according to
	[PEP440](https://www.python.org/dev/peps/pep-0440/#version-specifiers).
	Default: reads your package's Python compatibility from pyproject.toml
	(`project.requires-python`) or setup.cfg (`options.python_requires`). If not
	found, assumes the package is compatible with all versions of Python.
	!!! note
	Rather than using this option, it's recommended you set
	`project.requires-python` in `pyproject.toml` instead - that way, `pip`
	knows which wheels are compatible when installing.
	Example `pyproject.toml`:
	```toml
	[project]
	requires-python = ">=3.6"
	# aside - pyproject.toml also requires you specify build system
	# options, like this
	[build-system]
	requires = ["setuptools", "wheel"]
	build-backend = "setuptools.build_meta"
	```
	Example `setup.cfg`:
	```ini
	[options]
	python_requires = ">=3.6"
	```
	#### Examples
	```yaml
	CIBW_PROJECT_REQUIRES_PYTHON: ">=3.6"
	```

There are a few changes above, but generally I think it's better to emphasise that this is overriding the compatibility of the project being built, not choosing which builds to run.

Also, the header kinda needs to be an option name as it's used in the sidebar navigation.

[henryiii]

That's weird, I was sure I replaced this with your version (clearly remember the header change). Maybe I didn't push it? Edit: yes, I have it locally.

[joerick]

Another test with something more exotic like SpecifierSet(">=2.7.0, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*, !=3.4.*, !=3.5.*") might be valuable. Also something with a patch version, like SpecifierSet(">=2.7.9").

[henryiii]

I've never seen a patch version here, but there's no reason it wouldn't be valid. I've been wanting got add minor versions to the macOS identifiers; it would have made the generator script simpler and would have been more consistent (even though we currently never need more than the first two digits. Actually, if we include packaging now, we could keep them Version's and tidy up a bit.

[henryiii]

I understand including it for completeness, but a wheel tag does not include the patch version of Python it was built on; a wheel built with 3.9.1 can be loaded on 3.9.0. So I'm not sure how putting a patch version in your Python-Requires could ever be useful. Honestly, it might be better to always pretend to be on the ".0" patch version, so mistaken identifiers like >=2.7, !=3.0, !=3.1, !=3.2, !=3.3, !=3.4, !=3.5 would still work (I'd exactly match pip here; I'd have to see what it does; I'd assume it checks the full version).

[henryiii]

They use the full version: https://github.com/pypa/pip/blob/f30a02c7c26446d0e2b1b986ecbbe402795f8ccd/src/pip/_internal/utils/packaging.py#L21-L40

[henryiii]

Python-Requires tells you if the current version of Python is "new enough" to be used with your package or if you need to look back through older versions of a package; it does not tell you to go download a specific version like an install requirement could. Since Python does not add new features in patch releases (ignore 2.7), you are not likely to "drop" support for a patch version, there's no reason to put a patch version in here. (pip would go back and load an older version of your software that did support a patch version). How about, for now, we add .99 to any two digit version when checking this? Eventually we could try to compute an actual version, but there really isn't an answer for PyPy, and Linux will depend on the image, and you'd need to load it to detect what Python's where in it. >=3.6.3 and !=3.6.6 would both correctly work if we ask, when picking the identifiers that match, if 3.6.99 matches.

On windows we can use the real version, and adding it to macOS would be easy as well. PyPy and Linux could just check X.X.99.

As I've said before, this is about the Python language version, not the CPython version. And the language doesn't have patch versions. So I'd treat placing a patch version in Python-Requires here more as a user error.

[henryiii]

I bet these values are the ones used: https://www.pypy.org/compat.html

[henryiii]

And here's the value for 3.6: https://morepypy.blogspot.com/2020/11/pypy-733-triple-release-python-37-36.html

[YannickJadoul]

As I've said before, this is about the Python language version, not the CPython version. And the language doesn't have patch versions. So I'd treat placing a patch version in Python-Requires here more as a user error.

It shouldn't, no. But in practice, the only real existing standard for Python seems to be the CPython reference implementation (just ask PyPy how much they like this ;-) ). It's very rare/almost non-existant that the language itself changes, though, in patch releases, as I guess that's still not very semantically versioned.

[joerick]

The underlying question was resolved, but I still think-

Another test with something more exotic like SpecifierSet(">=2.7.0, !=3.0., !=3.1., !=3.2., !=3.3., !=3.4., !=3.5.") might be valuable. Also something with a patch version, like SpecifierSet(">=2.7.9").

[henryiii]

I could have sworn I added this...

[henryiii]

Okay, so the latest changes:

• I've pulled out config file reading to a class. If we eventually look for anything else in these files, this will extend nicely to that without having to read them multiple times. It's also where the check for existence now happens.
• Checks are made assuming the highest patch version, 3.6 checks as 3.6.99. Eventually we could refactor this to use the actual patch version if there's a need, but there's a couple of things that need to happen first (I'll report a possible idea in Configuration in pyproject.toml #547).
• I've pushed the docs changes based on my slight tweaking on the rewrite in ARCH and REQUIRES_PYTHON plan #528, maybe we can iterate from there?
• I needed the new option header from readthedocs to add it to the README, so haven't added it to the readme yet (TODO).

---

By the way, parsing the AST to pull out the value of setup(python_requires="") is really easy to do. This is basically what setuptools does for version=attr: ... (at least, they parse the AST, not sure how similar it is). Someone please tell me this is a bad idea...

#!/usr/bin/env python3

import ast
from typing import Optional, Dict
import click
from pathlib import Path


class Analyzer(ast.NodeVisitor):
    def __init__(self) -> None:
        self.requires_python: Optional[str] = None
        self.constants: Dict[str, str] = {}

    def visit_Assign(self, node: ast.Assign) -> None:
        for target in node.targets:
            if (
                isinstance(target, ast.Name)
                and isinstance(node.value, ast.Constant)
                and isinstance(node.value.value, str)
            ):
                self.constants[target.id] = node.value.value

    def visit_Call(self, node: ast.Call) -> None:
        self.generic_visit(node)
        if getattr(node.func, "id", "") == "setup":
            for kw in node.keywords:
                if kw.arg == "python_requires":
                    if isinstance(kw.value, ast.Constant):
                        self.requires_python = kw.value.value
                    elif isinstance(kw.value, ast.Name):
                        self.requires_python = self.constants.get(kw.value.id)


@click.command()
@click.argument(
    "filename", type=click.Path(exists=True, file_okay=False, resolve_path=True)
)
def main(filename: str) -> None:
    with open(Path(filename) / "setup.py") as f:
        tree = ast.parse(f.read())
    analyzer = Analyzer()
    analyzer.visit(tree)
    print(analyzer.requires_python)


if __name__ == "__main__":
    main()

Edit: Added simple assignment support to the above, too. This is what I was actually working on and why I had this basically ready :) : https://github.com/henryiii/setuptools-modernize/blob/main/src/setuptools_modernize/parse.py

[henryiii]

Oops, CIBW_ARCHS_LINUX is missing from the README, I tried copying the whole table out this time and noticed that. I'll make a PR that updates the known projects, and will fix that too. #552

[YannickJadoul]

To me, this file feels a bit over-engineered. It makes sense in a huge project, but I've always liked that cibuildwheel is reasonably straightforward, more or less reads front-to-back what's happening, and doesn't have a dozen files spread around.

I currently don't really see the advantage of this whole class over just keeping the old check for the existence of the file, and then a simple function utils.get_project_requires_python(package_dir). It's not like reading this file is going to cause cibuildwheel to become very slow/inefficient?

[YannickJadoul]

(For the record: I'm just being skeptical/questioning, here; I'm of course open to be convinced (or outvoted ;-) ) of the advantages of this construct, but I'm currently not sure this extra complexity, with custom exceptions etc, is worth it.)

[henryiii]

Personally, I like having small files with clear names, usually one per main class (helper classes can share files, as can support functions). If I see a class called ProjectFile, I'd like to have a file called projectfile.py. That way, you have a clear, focused set of includes instead of everything jumbled, you don't have items too heavily mixed, etc. utils.py is getting a bit crowded. I'd like to see a architecture.py with Architecture in it too, I think. Utils doesn't really read top to bottom, it's just a bag of unrelated utilities. Notice that some includes were removed from utils!

For logic, you want it to flow in one place. For classes, you want them in separate files, since they encapsulate logic. As long as the class is unit tested (which I could add) and you can trust it to work, then it simplifies the flow of logic where it is used.

Github even crosslinks if you click the class/function name, so I don't think it's too bad. :)

The design was general so that if we wanted to add more (like a setup.py parser, reading pyproject.toml to get config values, etc), it doesn't cause utils to grow, and all "project" related code could live together - existence checks and reading the files. Note that this always reads the files if present, while the old design was lazy and didn't read if nothing was needed (could use this and make it lazy, too, of course).

Custom exceptions are good! They let you be more specific when catching, and are extremely simple. But it could easily be made a method, though you always have to make a block to call sys.exit() somehow (unless you call it from within the function, but that's ugly, now you can't see where exits happen when reading main()).

[YannickJadoul]

If I see a class called ProjectFile, I'd like to have a file called projectfile.py.

That can make sense for a class this size, yes, but I'm doubting the overegineering of making this a class, vs. just the

if any_of_these_files_exist:
    exit_with_same_error_as_now()

# ... later, when reading options

requires_python = None if requires_python_str is None else SpecifierSet(requires_python_str)
if requires_python is None:
    requires_python = utils.get_requires_python(package_dir)

Especially the first part is a lot simpler than wrapping project_file = ProjectFile(package_dir) in a try-except and catching a new one-time-use exception. It's a simple check, so why not use if? I'm currently not convinced the extra complexity is worth it. It makes perfect sense in a large 50+ files project, but not for the current state of cibuildwheel (where we've been aiming to keep things transparent and procedural, I believe), to me.

[henryiii]

I think it makes sense to have a .exists() method that checks this, that would remove the exception and allow you to put the check wherever you wanted it to go.

I don't think utils is intended to read top to bottom.

[YannickJadoul]

I don't think utils is intended to read top to bottom.

No, utils isn't, ofc.

Maybe @joerick disagrees with me, and says this is more future-roof, though? It seems a bit heavy to me, right now, but ... I don't know what's coming later.

[henryiii]

Let me rewrite, and see if you like it better, I have some ideas. It was @joerick's original request that this be factored out, I had everything inline, originally.

[YannickJadoul]

Oh, I missed that. You mean #536 (comment)?

(Which does suggest "as a function", but OK, let's not split hairs too much :-P )

[henryiii]

"or something", and classes are better than functions :) unless it's just one shot; and this can at least combine existence check and config reading, and if we eventually add reading anything else from the config at all, it should be refactored to a class, so might as well do it here.

[joerick]

I haven't read all of the arguments above, but I don't think this warrants a class either. The trouble with classes is that they carry around hidden state, so they'd better be a good abstraction, otherwise you need to dig in and understand it and by then it's a net negative because you've got the inner workings of the class to worry about, as well as the task at hand.

I also think that generally, it's better to keep state in the filesystem, rather than in objects - that's a reason we don't have a Project object in cibuildwheel - it's just gonna read it from the filesystem anyway, so why try encapsulate that?

This coding style does feel unusual for some people (even sloppy), but we're optimising for lots of first-time readers, and they'll mostly be reading our code to understand why their build is failing. Literal is good.

As for the factoring out, a function seems good to me, because it can have an obvious name, that encapsulates the actual mechanism read_package_python_requires(path). But admittedly there is a little subjectivity to that!

[joerick]

I discovered in 07f972f that triple-backtick doesn't work inside the !!! note construct, but a 4-space indent does.

[henryiii]

But then you lose the syntax highlighting! Does ~~~ work?

[henryiii]

No, it doesn't. :'(

[henryiii]

Please take a look at the current version. It's still a class, but it has almost no state (just project_dir), and the existence check is optional and can be moved anywhere. I've also taken the one extra step to pull out python_requires if possible, I can back off on that. I've also got unit tests.

Docs need updates, ~~~ doesn't work, and they don't include setup.py (since I might remove the setup.py part).

[joerick]

Since we're into the style discussion, I still think this would be simpler as a function.

While the class is mostly stateless, as a reader reading this code, you can't tell it's stateless without reading the source of the class. So we'd still have to jump to that file to read what 'exists' and 'get_requires_python_str' mean.

Once we get there, we have to understand those property accessors, which reference more accessors, the dig function, and then finally the get_requires_python_str method, just to understand what's going on.

Going back to your original solution, but factored as a function-

def get_package_python_requires(package_dir: Path) -> Optional[str]:
    pyproject_toml = package_dir / 'pyproject.toml'
    try:
        info = toml.load(pyproject_toml)
        return info['project']['requires-python']
    except (FileNotFoundError, KeyError):
        pass

    # Read in from setup.cfg:options.python_requires
    setup_cfg = package_dir / 'setup.cfg'
    config = ConfigParser()
    try:
        config.read(setup_cfg)
        return config['options']['python_requires']
    except (FileNotFoundError, KeyError):
        pass
    
    # TODO: read from setup.py

It's just... so much clearer!

(There's also a smell in that the error message print('cibuildwheel: Could not find setup.py, setup.cfg or pyproject.toml at root of package' understands the mechanism of project_files.exists(). I'd consider that a 'leaky abstraction', though it could be solved by a custom exception, it's probably not worth it in this case.)

[YannickJadoul]

Yes. I agree with @henryiii that the original class design abstract things better in a large, complex project, but part of the charm of cibuildwheel (at least to me) is that it's almost "just a large script" that glues things together, rather than a full OO program.

[YannickJadoul]

(or at least, that's the current state and style, and it seems like that has worked and made things flexible and intuitive so far)

[henryiii]

On the python_requires check looking in setup.py, I've scanned all known projects, and here's the result. Green check means found (for setup.py, I've printed the AST discovery too). Grey check means it is set, but not able to be pulled from the AST with the current system (I've manually checked the setup.py's that do this, they all do something weird, like **kw or generating the python_requires with a " ".join). Red x means it is completely missing fro the file, but the file was there. Since it was right 16 times, and missed picking it up 6 times, and never "failed", I think it's a useful addition. Thoughts?

Note on the check: If a repo was not setup with setup.py in the root, it will report all missing. I think the was still a good sampling, though.

matplotlib/matplotlib:
  setup.py: ☑️
  setup.cfg: Not found
  pyproject.toml: Not found
joerick/pyinstrument_cext:
  setup.py: ❌
  setup.cfg: ❌
  pyproject.toml: Not found
aaugustin/websockets:
  setup.py: ✅ >=3.6.1
  setup.cfg: ❌
  pyproject.toml: Not found
YannickJadoul/Parselmouth:
  setup.py: ❌
  setup.cfg: ✅
  pyproject.toml: ❌
mayeut/pybase64:
  setup.py: ✅ >=3.5
  setup.cfg: ❌
  pyproject.toml: ❌
tommyod/KDEpy:
  setup.py: ❌
  setup.cfg: Not found
  pyproject.toml: Not found
autopilot-rs/autopy:
  setup.py: ❌
  setup.cfg: Not found
  pyproject.toml: Not found
pyrogram/tgcrypto:
  setup.py: ✅ ~=3.6
  setup.cfg: Not found
  pyproject.toml: Not found
twisted/twisted-iocpsupport:
  setup.py: ❌
  setup.cfg: ❌
  pyproject.toml: ❌
online-ml/river:
  setup.py: ✅ >=3.6.0
  setup.cfg: Not found
  pyproject.toml: Not found
PyAV-Org/PyAV:
  setup.py: ❌
  setup.cfg: ❌
  pyproject.toml: Not found
aiortc/aiortc:
  setup.py: ❌
  setup.cfg: ❌
  pyproject.toml: Not found
aiortc/aioquic:
  setup.py: ❌
  setup.cfg: ❌
  pyproject.toml: Not found
pikepdf/pikepdf:
  setup.py: ✅ >=3.6
  setup.cfg: ❌
  pyproject.toml: ❌
stfbnc/fathon:
  setup.py: ✅ >=3.5
  setup.cfg: Not found
  pyproject.toml: Not found
etesync/etebase-py:
  setup.py: ✅ >=3
  setup.cfg: Not found
  pyproject.toml: ❌
dimitern/xmlstarlet:
  setup.py: ✅ >=3.6.*
  setup.cfg: ❌
  pyproject.toml: Not found
bxlab/bx-python:
  setup.py: ☑️
  setup.cfg: ❌
  pyproject.toml: Not found
nedbat/coveragepy:
  setup.py: ☑️
  setup.cfg: ❌
  pyproject.toml: Not found
scikit-learn/scikit-learn:
  setup.py: ☑️
  setup.cfg: ❌
  pyproject.toml: ❌
scikit-image/scikit-image:
  setup.py: ✅ >=3.7
  setup.cfg: ❌
  pyproject.toml: ❌
google/neuroglancer:
  setup.py: ❌
  setup.cfg: Not found
  pyproject.toml: ❌
h5py/h5py:
  setup.py: ✅ >=3.7
  setup.cfg: Not found
  pyproject.toml: ❌
zeromq/pyzmq:
  setup.py: ☑️
  setup.cfg: Not found
  pyproject.toml: ❌
python-rapidjson/python-rapidjson:
  setup.py: ☑️
  setup.cfg: ❌
  pyproject.toml: Not found
mwilliamson/jq.py:
  setup.py: ✅ >=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*
  setup.cfg: Not found
  pyproject.toml: ❌
pybind/python_example:
  setup.py: ❌
  setup.cfg: Not found
  pyproject.toml: ❌
pybind/cmake_example:
  setup.py: ❌
  setup.cfg: ❌
  pyproject.toml: ❌
pybind/scikit_build_example:
  setup.py: ❌
  setup.cfg: Not found
  pyproject.toml: ❌
scikit-hep/iminuit:
  setup.py: ✅ >=3.6
  setup.cfg: ❌
  pyproject.toml: ❌
scikit-hep/pyjet:
  setup.py: ❌
  setup.cfg: ✅
  pyproject.toml: ❌
scikit-hep/numpythia:
  setup.py: ✅ >=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*, !=3.4.*
  setup.cfg: Not found
  pyproject.toml: ❌
scikit-hep/boost-histogram:
  setup.py: ❌
  setup.cfg: ✅
  pyproject.toml: ❌
Toblerity/rtree:
  setup.py: ❌
  setup.cfg: Not found
  pyproject.toml: ❌
giampaolo/psutil:
  setup.py: ✅ >=2.6, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*
  setup.cfg: Not found
  pyproject.toml: Not found
ets-labs/python-dependency-injector:
  setup.py: ❌
  setup.cfg: ❌
  pyproject.toml: Not found
pydata/numexpr:
  setup.py: ❌
  setup.cfg: Not found
  pyproject.toml: Not found
PyTables/PyTables:
  setup.py: ✅ >=3.6
  setup.cfg: ❌
  pyproject.toml: Not found
DataDog/dd-trace-py:
  setup.py: ✅ >=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*, !=3.4.*
  setup.cfg: ❌
  pyproject.toml: ❌
OpenNMT/Tokenizer:
  setup.py: Not found
  setup.cfg: Not found
  pyproject.toml: Not found
matrix-profile-foundation/matrixprofile:
  setup.py: ❌
  setup.cfg: ❌
  pyproject.toml: Not found
brentp/cyvcf2:
  setup.py: ❌
  setup.cfg: ❌
  pyproject.toml: Not found
OpenNMT/CTranslate2:
  setup.py: Not found
  setup.cfg: Not found
  pyproject.toml: Not found
mypyc/mypy_mypyc-wheels:
  setup.py: Not found
  setup.cfg: Not found
  pyproject.toml: Not found
czaki/imagecodecs_build:
  setup.py: Not found
  setup.cfg: Not found
  pyproject.toml: Not found
deepcharles/ruptures:
  setup.py: ❌
  setup.cfg: ✅
  pyproject.toml: ❌

[joerick]

Nice work! One question- How many of these use the visit_Assign part of the AST visitor? That was the only bit that gave me pause, as those assignments could be within an if, in which case they might not be executed. Also it's easier to explain if we say the detection only works if you pass python_requires like setup(python_requires='>=3.7')

[henryiii]

Python requires is static metadata that gets burned into the wheel (actually it might even be in the SDist metadata too?), so it's very unlikely to be in an if, and we are not currently checking to see if the setup itself is in an if. I didn't quite meant do this, but here's a run where I only look for a keyword called python_requires, ignoring what it is in (so dict(python_requires=...) works). This gets all but three.

We could tighten up the check to look for unnested "setup" functions only. It seems alignment is pretty rare; adding it to a dict is a bit more common.

matplotlib/matplotlib:
  setup.py: ☑️
  setup.cfg: Not found
  pyproject.toml: Not found
joerick/pyinstrument_cext:
  setup.py: ❌
  setup.cfg: ❌
  pyproject.toml: Not found
aaugustin/websockets:
  setup.py: ✅ >=3.6.1
  setup.cfg: ❌
  pyproject.toml: Not found
YannickJadoul/Parselmouth:
  setup.py: ❌
  setup.cfg: ✅
  pyproject.toml: ❌
mayeut/pybase64:
  setup.py: ✅ >=3.5
  setup.cfg: ❌
  pyproject.toml: ❌
tommyod/KDEpy:
  setup.py: ❌
  setup.cfg: Not found
  pyproject.toml: Not found
autopilot-rs/autopy:
  setup.py: ❌
  setup.cfg: Not found
  pyproject.toml: Not found
pyrogram/tgcrypto:
  setup.py: ✅ ~=3.6
  setup.cfg: Not found
  pyproject.toml: Not found
twisted/twisted-iocpsupport:
  setup.py: ❌
  setup.cfg: ❌
  pyproject.toml: ❌
online-ml/river:
  setup.py: ☑️
  setup.cfg: Not found
  pyproject.toml: Not found
PyAV-Org/PyAV:
  setup.py: ❌
  setup.cfg: ❌
  pyproject.toml: Not found
aiortc/aiortc:
  setup.py: ❌
  setup.cfg: ❌
  pyproject.toml: Not found
aiortc/aioquic:
  setup.py: ❌
  setup.cfg: ❌
  pyproject.toml: Not found
pikepdf/pikepdf:
  setup.py: ✅ >=3.6
  setup.cfg: ❌
  pyproject.toml: ❌
stfbnc/fathon:
  setup.py: ✅ >=3.5
  setup.cfg: Not found
  pyproject.toml: Not found
etesync/etebase-py:
  setup.py: ✅ >=3
  setup.cfg: Not found
  pyproject.toml: ❌
dimitern/xmlstarlet:
  setup.py: ✅ >=3.6.*
  setup.cfg: ❌
  pyproject.toml: Not found
bxlab/bx-python:
  setup.py: ✅ >=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*, !=3.4.*
  setup.cfg: ❌
  pyproject.toml: Not found
nedbat/coveragepy:
  setup.py: ✅ >=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*, !=3.4.*, <4
  setup.cfg: ❌
  pyproject.toml: Not found
scikit-learn/scikit-learn:
  setup.py: ✅ >=3.6
  setup.cfg: ❌
  pyproject.toml: ❌
scikit-image/scikit-image:
  setup.py: ✅ >=3.7
  setup.cfg: ❌
  pyproject.toml: ❌
google/neuroglancer:
  setup.py: ❌
  setup.cfg: Not found
  pyproject.toml: ❌
h5py/h5py:
  setup.py: ✅ >=3.7
  setup.cfg: Not found
  pyproject.toml: ❌
zeromq/pyzmq:
  setup.py: ✅ >=3.6
  setup.cfg: Not found
  pyproject.toml: ❌
python-rapidjson/python-rapidjson:
  setup.py: ☑️
  setup.cfg: ❌
  pyproject.toml: Not found
mwilliamson/jq.py:
  setup.py: ✅ >=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*
  setup.cfg: Not found
  pyproject.toml: ❌
pybind/python_example:
  setup.py: ❌
  setup.cfg: Not found
  pyproject.toml: ❌
pybind/cmake_example:
  setup.py: ❌
  setup.cfg: ❌
  pyproject.toml: ❌
pybind/scikit_build_example:
  setup.py: ❌
  setup.cfg: Not found
  pyproject.toml: ❌
scikit-hep/iminuit:
  setup.py: ✅ >=3.6
  setup.cfg: ❌
  pyproject.toml: ❌
scikit-hep/pyjet:
  setup.py: ❌
  setup.cfg: ✅
  pyproject.toml: ❌
scikit-hep/numpythia:
  setup.py: ✅ >=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*, !=3.4.*
  setup.cfg: Not found
  pyproject.toml: ❌
scikit-hep/boost-histogram:
  setup.py: ❌
  setup.cfg: ✅
  pyproject.toml: ❌
Toblerity/rtree:
  setup.py: ❌
  setup.cfg: Not found
  pyproject.toml: ❌
giampaolo/psutil:
  setup.py: ✅ >=2.6, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*
  setup.cfg: Not found
  pyproject.toml: Not found
ets-labs/python-dependency-injector:
  setup.py: ❌
  setup.cfg: ❌
  pyproject.toml: Not found
pydata/numexpr:
  setup.py: ❌
  setup.cfg: Not found
  pyproject.toml: Not found
PyTables/PyTables:
  setup.py: ✅ >=3.6
  setup.cfg: ❌
  pyproject.toml: Not found
DataDog/dd-trace-py:
  setup.py: ✅ >=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*, !=3.4.*
  setup.cfg: ❌
  pyproject.toml: ❌
OpenNMT/Tokenizer:
  setup.py: Not found
  setup.cfg: Not found
  pyproject.toml: Not found
matrix-profile-foundation/matrixprofile:
  setup.py: ❌
  setup.cfg: ❌
  pyproject.toml: Not found
brentp/cyvcf2:
  setup.py: ❌
  setup.cfg: ❌
  pyproject.toml: Not found
OpenNMT/CTranslate2:
  setup.py: Not found
  setup.cfg: Not found
  pyproject.toml: Not found
mypyc/mypy_mypyc-wheels:
  setup.py: Not found
  setup.cfg: Not found
  pyproject.toml: Not found
czaki/imagecodecs_build:
  setup.py: Not found
  setup.cfg: Not found
  pyproject.toml: Not found
deepcharles/ruptures:
  setup.py: ❌
  setup.cfg: ✅
  pyproject.toml: ❌

[YannickJadoul]

Shouldn't we assert for '-' in build_id?

[henryiii]

We haven't in the past, and there are unit tests that put really weird things in here. That's why this is here. :)

[YannickJadoul]

Aaaaaaaaah, right. Forgot about the unit tests again :-/
Hmmm, that's annoying, changing our code's assumptions to fit unit tests, but... changing unit tests to work around this is almost just as fishy.

[joerick]

mmmyeah, this does seem a little wrong... also the ValueError catch below. These shouldn't be hit in real use, so I think we should fix the unit tests. It looks like only test_build_selector in main_options_test.py?

[YannickJadoul]

Nitpicking, but why not just inline these? package_dir / 'setup.py' is just as expressive as setup_py, no?

[henryiii]

Just mimicking what was in main. There, pyproject_toml.exists() describes exactly what you mean (Does pyproject.toml exist?), while (package_dir / "pyproject.toml").exists()is ugly due to the parenthesis andpackage_dir.joinpath("pyproject.toml").exists()adds an extra term (and is not as common as/`).

[YannickJadoul]

Hmmm, myes, that's the one problem with pathlib :-(

Though you're not calling exists() here.

[YannickJadoul]

Not wrong, but the previous

if not any((package_dir / name).exists()
           for name in ["setup.py", "setup.cfg", "pyproject.toml"]):

was arguably a tiny bit more complex, but also less code duplication. (I have honestly no clue who wrote it.)

[henryiii]

Could even be:

if not any(p.exists() for p in {setup_py, setup_cfg, pyproject_toml}):

Don't have a strong preference, really, it's mostly different than what was there before because I had replaced it then rewrote it.

[YannickJadoul]

Then you still have the repetition, though, of package_dir / and setup.py/setup_py.

it's mostly different than what was there before because I had replaced it then rewrote it.

But I guessed so (and couldn't see a convincing reason for the change), so that's why I wanted to point it out.

[henryiii]

I went back to something closer to what we had before.

[henryiii]

Okay, only allowing a top-level function call, no assignments, and still gets us a pretty nice >50%:

./bin/inspect_all_known_projects.py
matplotlib/matplotlib:
  setup.py: ☑️
  setup.cfg: Not found
  pyproject.toml: Not found
joerick/pyinstrument_cext:
  setup.py: ❌
  setup.cfg: ❌
  pyproject.toml: Not found
aaugustin/websockets:
  setup.py: ✅ >=3.6.1
  setup.cfg: ❌
  pyproject.toml: Not found
YannickJadoul/Parselmouth:
  setup.py: ❌
  setup.cfg: ✅
  pyproject.toml: ❌
mayeut/pybase64:
  setup.py: ✅ >=3.5
  setup.cfg: ❌
  pyproject.toml: ❌
tommyod/KDEpy:
  setup.py: ❌
  setup.cfg: Not found
  pyproject.toml: Not found
autopilot-rs/autopy:
  setup.py: ❌
  setup.cfg: Not found
  pyproject.toml: Not found
pyrogram/tgcrypto:
  setup.py: ✅ ~=3.6
  setup.cfg: Not found
  pyproject.toml: Not found
twisted/twisted-iocpsupport:
  setup.py: ❌
  setup.cfg: ❌
  pyproject.toml: ❌
online-ml/river:
  setup.py: ☑️
  setup.cfg: Not found
  pyproject.toml: Not found
PyAV-Org/PyAV:
  setup.py: ❌
  setup.cfg: ❌
  pyproject.toml: Not found
aiortc/aiortc:
  setup.py: ❌
  setup.cfg: ❌
  pyproject.toml: Not found
aiortc/aioquic:
  setup.py: ❌
  setup.cfg: ❌
  pyproject.toml: Not found
pikepdf/pikepdf:
  setup.py: ☑️
  setup.cfg: ❌
  pyproject.toml: ❌
stfbnc/fathon:
  setup.py: ☑️
  setup.cfg: Not found
  pyproject.toml: Not found
etesync/etebase-py:
  setup.py: ✅ >=3
  setup.cfg: Not found
  pyproject.toml: ❌
dimitern/xmlstarlet:
  setup.py: ✅ >=3.6.*
  setup.cfg: ❌
  pyproject.toml: Not found
bxlab/bx-python:
  setup.py: ☑️
  setup.cfg: ❌
  pyproject.toml: Not found
nedbat/coveragepy:
  setup.py: ✅ >=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*, !=3.4.*, <4
  setup.cfg: ❌
  pyproject.toml: Not found
scikit-learn/scikit-learn:
  setup.py: ☑️
  setup.cfg: ❌
  pyproject.toml: ❌
scikit-image/scikit-image:
  setup.py: ☑️
  setup.cfg: ❌
  pyproject.toml: ❌
google/neuroglancer:
  setup.py: ❌
  setup.cfg: Not found
  pyproject.toml: ❌
h5py/h5py:
  setup.py: ✅ >=3.7
  setup.cfg: Not found
  pyproject.toml: ❌
zeromq/pyzmq:
  setup.py: ✅ >=3.6
  setup.cfg: Not found
  pyproject.toml: ❌
python-rapidjson/python-rapidjson:
  setup.py: ☑️
  setup.cfg: ❌
  pyproject.toml: Not found
mwilliamson/jq.py:
  setup.py: ✅ >=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*
  setup.cfg: Not found
  pyproject.toml: ❌
pybind/python_example:
  setup.py: ❌
  setup.cfg: Not found
  pyproject.toml: ❌
pybind/cmake_example:
  setup.py: ❌
  setup.cfg: ❌
  pyproject.toml: ❌
pybind/scikit_build_example:
  setup.py: ❌
  setup.cfg: Not found
  pyproject.toml: ❌
scikit-hep/iminuit:
  setup.py: ✅ >=3.6
  setup.cfg: ❌
  pyproject.toml: ❌
scikit-hep/pyjet:
  setup.py: ❌
  setup.cfg: ✅
  pyproject.toml: ❌
scikit-hep/numpythia:
  setup.py: ✅ >=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*, !=3.4.*
  setup.cfg: Not found
  pyproject.toml: ❌
scikit-hep/boost-histogram:
  setup.py: ❌
  setup.cfg: ✅
  pyproject.toml: ❌
Toblerity/rtree:
  setup.py: ❌
  setup.cfg: Not found
  pyproject.toml: ❌
giampaolo/psutil:
  setup.py: ☑️
  setup.cfg: Not found
  pyproject.toml: Not found
ets-labs/python-dependency-injector:
  setup.py: ❌
  setup.cfg: ❌
  pyproject.toml: Not found
pydata/numexpr:
  setup.py: ❌
  setup.cfg: Not found
  pyproject.toml: Not found
PyTables/PyTables:
  setup.py: ☑️
  setup.cfg: ❌
  pyproject.toml: Not found
DataDog/dd-trace-py:
  setup.py: ☑️
  setup.cfg: ❌
  pyproject.toml: ❌
OpenNMT/Tokenizer:
  setup.py: Not found
  setup.cfg: Not found
  pyproject.toml: Not found
matrix-profile-foundation/matrixprofile:
  setup.py: ❌
  setup.cfg: ❌
  pyproject.toml: Not found
brentp/cyvcf2:
  setup.py: ❌
  setup.cfg: ❌
  pyproject.toml: Not found
OpenNMT/CTranslate2:
  setup.py: Not found
  setup.cfg: Not found
  pyproject.toml: Not found
mypyc/mypy_mypyc-wheels:
  setup.py: Not found
  setup.cfg: Not found
  pyproject.toml: Not found
czaki/imagecodecs_build:
  setup.py: Not found
  setup.cfg: Not found
  pyproject.toml: Not found
deepcharles/ruptures:
  setup.py: ❌
  setup.cfg: ✅
  pyproject.toml: ❌

[YannickJadoul]

Curious intermezzo, slightly off-topic question for @henryiii: is there an advantage to using pyproject.toml for this rather than setup.cfg?

[henryiii]

is there an advantage to using pyproject.toml for this rather than setup.cfg?

Well, this will be the canonical location for this information, and will be following a standard (PEP 621), rather than being tool specific like setup.cfg or [tool.*] in pyproject.toml. Things like the package name can be specified in one place and picked up regardless of whether you use setuptools, poetry, flit, enscons, or something else.

However, today, there isn't an implementation of a major tool that will pick this up from pyproject.toml yet. Setuptools will take time to add it (Marchish?), and flit/poetry are kind of balking at going first (after they were trying to lead originally...). At least, in poetry's case, they like their complicated requirement system better than the standard one. I personally still see their system as solving a problem that didn't exist, and being more complex than required, so looking forward to being able to use the standard location and spelling. :)

[henryiii]

Fiddled with the MyPy configuration quite a bit, part of the 0.800 update and it's extra fixes for namespace packaging causing our workarounds for multiple conftest.py's. I've made them actual, normal packages, which fixes the import check from MyPy as well as lets us do a relative import in the one test that actually imports from a conftest.py. Should be much better now, and we can control the test mypy coverage properly now. The bin directory is now covered in pre-commit under the 3.7 language level; you have to run mypy --python-version 3.7 bin manually to test it by hand. Now that I think of it, maybe we should set the language level in the config to 3.7 and then add the 3.6 to pre-commit only...

[henryiii]

Enjoy the 3.7 + mypy 0.800 code in the new script. :)

[henryiii]

Looks like we were not the first to pick up PEP 621: https://frostming.com/2021/01-22/introducing-pdm/#pdm----a-new-python-package-manager-and-workflow-tool

[joerick]

Cool, I think this is pretty close.

[joerick]

The underlying question was resolved, but I still think-

Another test with something more exotic like SpecifierSet(">=2.7.0, !=3.0., !=3.1., !=3.2., !=3.3., !=3.4., !=3.5.") might be valuable. Also something with a patch version, like SpecifierSet(">=2.7.9").

[joerick]

mmmyeah, this does seem a little wrong... also the ValueError catch below. These shouldn't be hit in real use, so I think we should fix the unit tests. It looks like only test_build_selector in main_options_test.py?

[joerick]

Looks good!

[henryiii]

@YannickJadoul Would you like to do a once-over too?

[YannickJadoul]

@YannickJadoul Would you like to do a once-over too?

I kind of followed this along the way, but let me quickly check!

[YannickJadoul]

Slightly surprised this is more code than I had expected, but yeah, I think I had already made my complaints along the way :-)

Good job, as ever, @henryiii! :-)