nox-poetry icon indicating copy to clipboard operation
nox-poetry copied to clipboard

Support installing Poetry dependency groups

Open Niicck opened this issue 2 years ago • 13 comments

Closes https://github.com/cjolowicz/nox-poetry/issues/663 Closes https://github.com/cjolowicz/nox-poetry/issues/977 Closes https://github.com/cjolowicz/nox-poetry/issues/873

This PR adds a install_groups method to nox_poetry Session proxy. It has the same interface as session.install except that you pass in the dependency groups that you want to install. This feature requires poetry >=1.2.0, but the rest of the code is compatible with <1.2.0, there are no breaking changes.

Example usage: session.install_groups("lint", "test")

Implementation Notes

  • install_groups uses poetry export's --only flag to just get the requirements for the groups you specify. They are then installed via pip install -r.
  • Creating install_groups as a new method on the Session class was the cleanest way to implement this feature. My first approach followed this suggestion of adding poetry_groups as a kwarg of install. But this created confusing interactions between installing individual dependencies and installing entire group dependencies. The initial support PR had the right approach, so I sprang off from there.
  • I had initially dropped python3.7 and poetry <1.2.0 support, but I re-added it since we still want to support them.
  • The filename generated by _PoetrySession.export_requirements is now more explicit. If we're generating a constraints file to only install individual packages, then export_requirements will generate a constraints.txt. If we're creating a requirements file that is intended to install the entirety of select poetry dependency groups, then export_requirements will generate a requirements file with the format group1,group2-requirements.txt. The requirements.txt file is prefixed with the groups so that if our user inputted install_groups change, we'll regenerate the requirements.txt file, even if the poetry.lock hash hasn't changed.
  • In order to make our functional tests faster, I only added one session that uses poetry v1.2.0. We could add more test session combinations if that would be desirable.

Niicck avatar Feb 07 '23 21:02 Niicck

@Niicck I think this also closes #873 due to the changes in src/nox_poetry/poetry.py unless I'm mistaken

samypr100 avatar Feb 08 '23 15:02 samypr100

@Niicck from importlib import metadata is python 3.8+ only, but since importlib-metadata is a dependency (from nox) its possible to use import importlib_metadata as metadata on 3.7 and below as a replacement that way we can keep 3.7 compatibility as stated in the docs for this project

e.g.

try:
    from importlib import metadata
except ImportError:
    import importlib_metadata as metadata

Edit: Only version v3.x+ of importlib-metadata has the py.typed, if you get issues w/ mypy on 3.7 # type: ignore's might be needed if the importlib-metadata package is not high enough (via the dev dependencies which resolves to 1.7.0)

samypr100 avatar Feb 08 '23 15:02 samypr100

@samypr100 Big thanks for that check. Tests and mypy are now passing for 3.7

Niicck avatar Feb 08 '23 18:02 Niicck

Edit: There is now an MR for this:

  • https://github.com/python-poetry/poetry/pull/7529

As a slight tangent, when I was playing around with --with and --only I noticed this Poetry bug:

  • https://github.com/python-poetry/poetry/issues/7303

If anyone was intrepid, fixing that upstream would make using this nox-poetry feature less silently error prone to typos.

johnthagen avatar Feb 12 '23 13:02 johnthagen

@cjolowicz Would you be able to review this MR?

johnthagen avatar Feb 18 '23 16:02 johnthagen

Hello friends. I'm going to be offline for the next couple of months. If there are change requests in future reviews, someone else is welcome to address them and add to this PR.

In the meantime, if people want to install poetry groups using nox, I have a workaround install_poetry_groups function that I use with vanilla nox. It's not unit-tested and it's not as robust as this PR's implementation, but it's functional enough.

Here's the code for that helper function with a couple examples:

def install_poetry_groups(session, *groups: str) -> None:
    """Install dependencies from poetry groups.

    Using this as s workaround until my PR is merged in:
    https://github.com/cjolowicz/nox-poetry/pull/1080
    """
    with tempfile.NamedTemporaryFile() as requirements:
        session.run(
            "poetry",
            "export",
            *[f"--only={group}" for group in groups],
            "--format=requirements.txt",
            "--without-hashes",
            f"--output={requirements.name}",
            external=True,
        )
        session.install("-r", requirements.name)

@nox.session(python=PYTHON_VERSIONS)
@nox.parametrize("django_version", DJANGO_VERSIONS)
def test(session: nox.Session, django_version: str) -> None:
    """Run the pytest suite."""
    args = session.posargs
    install_poetry_groups(session, "main", "test", "dev", "coverage")
    session.install(f"django=={django_version}")
    try:
        session.run("coverage", "run", "--parallel", "-m", "pytest", *args)
    finally:
        if session.interactive:
            session.notify("coverage", posargs=[])

@nox.session(python=PYTHON_VERSIONS[0])
def coverage(session: nox.Session) -> None:
    """Produce the coverage report.

    Combines the results from all test runs from all versions of python. This is because
    some logic branches may only apply to certain versions of python - we want to avoid
    false negative coverage failures when those branches aren't covered by pytest runs
    from other python versions.
    """
    args = session.posargs or ["report"]
    install_poetry_groups(session, "coverage")

    if not session.posargs and any(Path().glob(".coverage.*")):
        session.run("coverage", "combine")

    session.run("coverage", *args)

Niicck avatar Mar 22 '23 14:03 Niicck

Any hope to merge this soon @cjolowicz ?

RomainBrault avatar Apr 03 '23 15:04 RomainBrault

@cjolowicz Curious, is nox-poetry something you'd like to continue to maintain, or are you using helper scripts like what you've mentioned before (https://github.com/cjolowicz/nox-poetry/issues/663#issuecomment-1423240512). Would you like to pass nox-poetry off to others in the community who'd like to continue to move it forward?

johnthagen avatar Apr 21 '23 19:04 johnthagen

The minimum should be avoiding warnings (#873) and anticipate future breakage(s).

RomainBrault avatar Apr 22 '23 20:04 RomainBrault

@cjolowicz Curious, is nox-poetry something you'd like to continue to maintain, or are you using helper scripts like what you've mentioned before (#663 (comment)). Would you like to pass nox-poetry off to others in the community who'd like to continue to move it forward?

@johnthagen I've been swamped with other work, but I expect to circle back to this project in the coming few weeks. I'll do my best to come up with a sustainable model for maintaining this package going forward. That might involve passing it on to others or reducing the bus factor.

cjolowicz avatar Apr 23 '23 10:04 cjolowicz

@Niicck FYI, there are some merge conflicts in this branch now, likely from

  • #1139

johnthagen avatar Jul 09 '23 12:07 johnthagen

@Niicck I was curious if you still had any interest in getting this branch back ready for a merge. Would ❤️ this feature.

johnthagen avatar Oct 04 '23 19:10 johnthagen

@Niicck I was curious if you still had any interest in getting this branch back ready for a merge. Would ❤️ this feature.

If @cjolowicz is interested in approving this PR/feature, then yes I'd be happy to bring the branch back into merge shape. If not, then I won't put any more development time into something that we're not intending to merge.

Niicck avatar Oct 04 '23 20:10 Niicck