Add buildpack for pyproject.toml to configure container image

[rgaiacs]

Related to #1427

This is not yet ready for review!

The code is based on the Pipfile buildpack. We probably want to refactor some portions to avoid code duplication.

[rgaiacs]

577cb1b implements a minimal working version. It was tested with https://github.com/rgaiacs/binder-examples-pyproject.

Required changes before merge

• To parse the pyproject.toml, we require tomllib that is part of Python 3.11 standard library. This means that we will have to change the minimal version of Python required by repo2docker to Python 3.11 or later.

Changes that can be done in another pull request

• Document the use of pyproject.toml
• Migrate https://github.com/rgaiacs/binder-examples-pyproject to https://github.com/binder-examples/pyproject

[rgaiacs]

All tests are passing after support to Python < 3.11 was dropped.

[manics]

We probably want to refactor some portions to avoid code duplication.

I've got a PR that refactors the handling of runtime.txt
#1428

[rgaiacs]

I rebased this pull request and I made minor changes to reduce the code duplication. It need to be reviewed again.

[rgaiacs]

Can we merge this? Or should we talk about it in jupyterhub/team-compass#814?

[minrk]

To parse the pyproject.toml, we require tomllib that is part of Python 3.11 standard library.

We can depend on tomli for Python < 3.11, which is the package that became tomllib in 3.11, so we should be able to:

try:
    import tomllib
except ImportError:
    import tomli as tomllib

without losing any supported environments.

I think reverting the baseline version change and depending on tomli, and fixing detection of pyproject.toml contents, I think this should be all set.

[rgaiacs]

I addressed the comments from @minrk

• use tomli to continue supporting Python < 3.11
• check for project and build-system.requires in the pyproject.toml

I also added

• single test to check if pyproject.toml were used
• list pyproject.toml in the documentation

I suggest to add more tests in another pull request.

[minrk]

This implementation looks great to me. My only comment is that I don't think we necessarily need a new buildpack to do this - the assemble steps, etc. should be identical to when setup.py is present. The only effect it should have is detecting the Python version, so it seems to me that it would be simpler to keep everything consistent if this logic is added to the python buildpack, rather than a whole new buildpack that should do all the same things.

But we can also do that as a simplification later. What do you think?

[rgaiacs]

the assemble steps, etc. should be identical to when setup.py is present.

I don't like code duplication but my line of thoughts was "when we decided to drop support to setup.py, we can remove the setup.py buildpack instead of review all the code with setup.py and pyproject.toml.

Tomorrow, I will refactor the code so that there are only one Python buildpack.

[minrk]

we can remove the setup.py buildpack

We don't have a setup.py buildpack. setup.py is one small part of the python buildpack. All of the steps we take if setup.py is detected are identical to the steps taken if pyproject.toml is detected (we never invoke setup.py directly, only pip install .).

So the real change here ought to be changing the "should we run pip install ." in the Python buildpack from "setup.py exists" to a more general "is it a python package?" which checks:

• setup.py exists, OR
• pyproject exists and contains project info

Then the only other change is detecting Python version from project info, I think, at a lower priority than runtime.txt, binder dir, etc.

The rest of python buildpack should be unmodified, right?

[rgaiacs]

b755b7a merges PyprojectBuildPack into PythonBuildPack.

@minrk how much safe net we need for the users? The code only checks for the project field in detect(). If a user has pyproject.toml without the project field and setup.py in the repository, I believe that repo2docker will fail.

[minrk]

I'm not sure I quite follow - The detect you have checks for the presence build-system and project, which is what we should do. This same condition should be applied to whether to invoke pip install.

We could isolate this into a single _is_python_package method which checks setup.py present OR pyproject.toml contains build-system, and use that where we need this information, which it looks like is in two places.

This is because pyproject.toml can be used to configure tools (ruff, black, pytest, etc.) for repos that are not packages. So it's not so much a safety net as it is an accuracy of the check: pyproject.toml alone is insufficient information to decide if pip install should be called, you have to check its contents to see if it's a package or another kind of project directory.

[rgaiacs]

Sorry for the delay to pick this one again.

• rebase pull request to resolve conflicts from require Python 3.10 #1481
• use packaging for version detection. Thanks to @minrk for the suggestion.

We should wait the end of JupyterCon to merge this.

[yuvipanda]

Excited for this to land, and it looks pretty good to me! Do you think your last comments have been fully addressed, @minrk?

[minrk]

So sorry, I wrote this review 2 weeks ago, but failed to submit it. I think it's almost there, but we need to always check the contents of pyproject.toml before using it, which isn't quite done yet.

[minrk]

Suggested change

	# Either not a Python runtime (e.g. R, which subclasses this)
	# Runtime specified, but not Python (e.g. R, which subclasses this)

[minrk]

This still needs to check for [build-system].

I think it's probably a good idea to have a cached method:

@lru_cache
def _is_python_package(self):
    if self.binder_dir:
        return False
    if os.path.exists("setup.py"):
        return True
    if os.path.exists("pyproject.toml"):
            with open(pyproject_toml, "rb") as _pyproject_file:
                pyproject = tomllib.load(_pyproject_file)

            if (
                ("project" in pyproject)
                and ("build-system" in pyproject)
            ):
                return True
    return False

because we need the exact same check in a few places, and they aren't currently in sync.

[minrk]

If the default version doesn't satisfy, we should probably pick the lowest version available rather than error, right? But only if runtime.txt is not specified?

# pseudocode
if runtime_version not in pyproject_python_specifier:
    if version:
        raise ValueError(
                        f"Python version {version} in runtime.txt does not satisfy {pyproject_python_specifier} in pyproject.toml."
                    )
    else:
        # version not specified in runtime.txt, pick the lowest available satisfying version
        for python in sorted(self.major_pythons):
            if python in pyproject_python_specifier:
                runtime_version = python
                break
        else:
            raise ValueError(f"Unable to find a supported Python version satisfying {pyproject_python_specifier}")

[minrk]

I think this part can be a later refinement, if it seems too complex to address right now. This isn't a regression, at least.

[minrk]

If we decide to go with my suggestion below, we should perhaps only show this warning if we don't end up picking a version based on pyproject.toml.