Unable to pass a function to `session.notify` (misleading docstring?)
Current Behavior
The docs and docstring for session.notify say that the target "may be specified as the appropriate string [...] or using the function object". I assumed this meant you could pass a @nox.session-decorated function into session.notify. However, the type annotation (target: str | SessionRunner) doesn't permit this, and indeed it doesn't work:
nox > Running session notify
nox > Creating virtual environment (virtualenv) using python.exe in .nox\notify
notify
nox > Session notify raised exception ValueError('Session <nox._decorators.Func object at 0x0000028516004310> not found.')
Traceback (most recent call last):
File "C:\Users\...\venv\Lib\site-packages\nox\sessions.py", line 761, in execute
self.func(session)
File "C:\Users\...\venv\Lib\site-packages\nox\_decorators.py", line 75, in __call__
return self.func(*args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^
File "C:\Users\...\noxfile.py", line 22, in notify
session.notify(target)
File "C:\Users\...\venv\Lib\site-packages\nox\sessions.py", line 641, in notify
self._runner.manifest.notify(target, posargs)
File "C:\Users\...\venv\Lib\site-packages\nox\manifest.py", line 319, in notify
raise ValueError(f"Session {session} not found.")
ValueError: Session <nox._decorators.Func object at 0x0000028516004310> not found.
Am I misinterpreting the docstring, or is this not actually supported?
Expected Behavior
Passing a function should run that session as documented.
Steps To Reproduce
With this noxfile:
import nox
@nox.session
def target(session: nox.Session):
print("target")
@nox.session
def notify(session: nox.Session):
print("notify")
session.notify(target)
Run nox -s notify. The above error is raised.
For comparison, this noxfile works as expected:
import nox
@nox.session
def target(session: nox.Session):
print("target")
@nox.session
def notify(session: nox.Session):
print("notify")
session.notify("target") # this is the only difference
Environment
- OS: Windows 11
- Python: 3.11.0
- Nox: 2023.4.22
Anything else?
It seems like this behaviour/documentation may have been present since all the way back when notify was introduced (d6b3a5ad47d4cc616c821ec1bef82a69ba774fd0)?
Session.notify:
Args:
target (Union[str, Callable]): The session to be notified. This
may be specified as the appropropriate string or using
the function object.
Manifest.notify:
Args:
session (Union[str, ~nox.session.Session]): The session to be
enqueued.
target appears to be passed verbatim to session, so I'm not sure if this ever actually worked.
The simplest fix would be to compare s.func == session here:
https://github.com/wntrblm/nox/blob/5c82dc553bd04ee017784fc16193da0b35a44ab6/nox/manifest.py#L313
If we go this way, the parameter type would need to change. I don't think there's a reasonable way to get hold of the SessionRunner object in the Noxfile. I'm not sure how we should type the parameter Session.notify and Manifest.notify. Either of these maybe?
https://github.com/wntrblm/nox/blob/5c82dc553bd04ee017784fc16193da0b35a44ab6/nox/registry.py#L26
https://github.com/wntrblm/nox/blob/5c82dc553bd04ee017784fc16193da0b35a44ab6/nox/_decorators.py#L59
They're currently not part of the public interface.
We should also consider changing the docstring and type to simply disallow this. It never worked, and I'm not sure there's a use case for it.
update: On second thoughts, I think there is a case for allowing this. If you pass the function instead of a string, typos can be caught at type-check time (in other words, you get a wiggly line in your editor). It doesn't constrain the order of the function definitions because the notify call happens in the function scope.
IMO, we should change the docstring for now, and not add this until after working on any reworks planned, like #631 or https://github.com/wntrblm/nox/issues/167#issuecomment-2040967680, as it might complicate those. Long term, I think it makes sense.
If you pass the function instead of a string, typos can be caught at type-check time (in other words, you get a wiggly line in your editor).
This would be great! Here is a workaround I'm currently using to get this:
@nox.session()
def some_session(session: nox.Session):
session.notify(some_other_session.__name__)
Perhaps this could suggested in the docstring if useful?
One downside of using
session.notify(some_other_session.__name__)
Is that if you type check your noxfile.py, mypy will raise
error: "Func" has no attribute "__name__" [attr-defined]
Could nox.session's type information be updated to including __name__: str?
Related
- https://stackoverflow.com/a/62800625