First impressions & suggestions

[brechtm]

There are some minor things that occurred to me when taking my first steps with nox-poetry:

1. the distribution_format argument to installroot needs to be of type DistributionFormat; it would be handy if it would accept strings ('sdist', 'wheel') as well.

2. installroot requires distribution_format to be passed as a keyword argument. I didn't know this was possible and this is the first time I encountered this so it confused me.

3. Perhaps a (slightly) nicer way to integrate with nox is to offer a replacement for nox.session. Then one could do:

    from nox_poetry import session, SDIST
    
    @session()
    def unit(session):
        session.installroot(SDIST)
        session.install('pytest')
        session.run('pytest', 'tests')

[cjolowicz]

Perhaps a (slightly) nicer way to integrate with nox is to offer a replacement for nox.session.

I like this idea. Something like this could work:

def session(*args: Any, **kwargs: Any) -> Any:
    if not args:
        return functools.partial(session, **kwargs)

    [function] = args

    @functools.wraps(function)
    def wrapper(session: nox.Session) -> None:
        proxy = Session(session)
        return function(proxy)

    return nox.session(wrapper, **kwargs)


class Session:
    def __init__(self, session: nox.Session) -> None:
        self._session = session

    def __getattr__(self, name: str) -> Any:
        return getattr(self._session, name)

    def install(self, *args: str, **kwargs: Any) -> None:
        return install(self._session, *args, **kwargs)

What bothers me though is that users would lose the ability to type-check their noxfiles. That is, unless I repeat nox's entire session interface instead of using __getattr__.

[brechtm]

This works, but still requires monkey-patching nox:

import nox
import nox_poetry


PYTHONS = ['3.6', '3.7', '3.8', '3.9', '3.10', 'pypy3.6', 'pypy3.7']

DEPENDENCIES = ['pytest', 'pytest-xdist', 'pytest-cov',
                'coverage', 'Sphinx']


class Session(nox.Session):
    def installroot(self, distribution_format=nox_poetry.WHEEL):
        nox_poetry.installroot(self, distribution_format=distribution_format)

    def install(self, *args, **kwargs):
        nox_poetry.install(self, *args, **kwargs)


nox.sessions.Session = Session


@nox.session(python=PYTHONS)
def unit(session):
    session.installroot(nox_poetry.SDIST)
    session.install(*DEPENDENCIES)
    session.run('python', 'run_tests.py', *session.posargs, 'tests')

Perhaps Nox could accommodate by providing extensions points?

EDIT: Or you could wrap the nox.Session object in your code above using the decorator pattern (I think that implementation of the patterns may be flawed though).

[cjolowicz]

Here's a first attempt at this: #259

This works, but still requires monkey-patching nox

I was thinking that users would simply import session from nox_poetry:

import nox_poetry


PYTHONS = ['3.6', '3.7', '3.8', '3.9', '3.10', 'pypy3.6', 'pypy3.7']

DEPENDENCIES = ['pytest', 'pytest-xdist', 'pytest-cov',
                'coverage', 'Sphinx']


@nox_poetry.session(python=PYTHONS)
def unit(session):
    session.installroot(nox_poetry.SDIST)
    session.install(*DEPENDENCIES)
    session.run('python', 'run_tests.py', *session.posargs, 'tests')

Perhaps Nox could accommodate by providing extensions points?

Yes, I actually experimented with making nox pluggable here, but it requires a lot more work and thought. I think I implemented a bunch of other hooks to see where this could go, but never pushed them. The downstream changes are in #119.

[cjolowicz]

Oh now I see what you meant, using inheritance to allow type checking without duplicating the interface.

For now, I decided to use type stubs. At least this keeps the duplication and coupling out of the runtime code.

[cjolowicz]

I addressed some of your other suggestions regarding DistributionFormat in #261. Do you find the interface more usable with these changes?

Update: This was split into #261 and #265

I'm still unsure about your suggestion regarding the use of keyword-only arguments, i.e. allowing installroot("sdist") instead of installroot(distribution_format="sdist"). The second form is admittedly quite wordy. On the other hand, I like how keyword-only arguments make it easier to extend an API, and how they make function calls more explicit. So I tend to use them when the parameter is, informally, about the "how" rather than the "what".

[brechtm]

Perhaps Nox could accommodate by providing extensions points?

Yes, I actually experimented with making nox pluggable here, but it requires a lot more work and thought. I think I implemented a bunch of other hooks to see where this could go, but never pushed them. The downstream changes are in #119.

Maybe the Nox maintainers would be open to integrating nox-poetry into Nox, seeing as Poetry seems to be gaining significant traction?

Oh now I see what you meant, using inheritance to allow type checking without duplicating the interface.

Sorry, I was thinking out loud and not making much sense. The main idea was to subclass nox.Session and simply extend/override. I was assuming the session decorator would allow using the nox-poetry Session class, but alas the Nox architecture doesn't allow for that.

I addressed some of your other suggestions regarding DistributionFormat in #261. Do you find the interface more usable with these changes?

❤️ Love it!

I'm still unsure about your suggestion regarding the use of keyword-only arguments, i.e. allowing installroot("sdist") instead of installroot(distribution_format="sdist"). The second form is admittedly quite wordy. On the other hand, I like how keyword-only arguments make it easier to extend an API, and how they make function calls more explicit. So I tend to use them when the parameter is, informally, about the "how" rather than the "what".

That makes perfect sense. Encountering this for the first time just confused me. Perhaps add something to the README if more people complain about is? As for the wordy distribution_format, you could consider renaming it to format.

[cjolowicz]

Maybe the Nox maintainers would be open to integrating nox-poetry into Nox, seeing as Poetry seems to be gaining significant traction?

Personally, I'd prefer the Nox API to remain based on packaging standards, possibly with extension points for specific tool support. Also, I feel that nox-poetry needs to mature a little more before I would suggest changes to Nox. Poetry is also still evolving quite significantly, and I'm still hoping for a situation where Poetry's feature set would render nox-poetry superfluous.

[re: keyword-only arguments] Perhaps add something to the README if more people complain about is?

Sounds good.

As for the wordy distribution_format, you could consider renaming it to format.

Yes, I've considered that. It's a good name in the context of build_package, a little less so in the context of installroot. But acceptable, I guess. However, we'd need to deprecate the old keyword argument. There's some prior art in the codebase for that.

PR(s) welcome :)