From 5cfccc48793734f84c9623a50ef36256486d48d7 Mon Sep 17 00:00:00 2001 From: DeltaDizzy Date: Sun, 28 Apr 2024 23:08:25 -0400 Subject: [PATCH 1/4] add deprecation and docs --- commands2/command.py | 7 ++++--- commands2/deferredcommand.py | 12 +++++++++--- commands2/proxycommand.py | 22 +++++++++++++++++++--- 3 files changed, 32 insertions(+), 9 deletions(-) diff --git a/commands2/command.py b/commands2/command.py index 5be9d733..5eaa82a6 100644 --- a/commands2/command.py +++ b/commands2/command.py @@ -344,9 +344,10 @@ def repeatedly(self) -> RepeatCommand: def asProxy(self) -> ProxyCommand: """ - Decorates this command to run "by proxy" by wrapping it in a ProxyCommand. This is - useful for "forking off" from command compositions when the user does not wish to extend the - command's requirements to the entire command composition. + Decorates this command to run "by proxy" by wrapping it in a ProxyCommand. Use this for + "forking off" from command compositions when the user does not wish to extend the command's + requirements to the entire command composition. ProxyCommand has unique implications and + semantics, see the `WPILib docs `_ for a full explanation. :returns: the decorated command """ diff --git a/commands2/deferredcommand.py b/commands2/deferredcommand.py index 992963d9..3115290b 100644 --- a/commands2/deferredcommand.py +++ b/commands2/deferredcommand.py @@ -11,9 +11,9 @@ class DeferredCommand(Command): """ - Defers Command construction to runtime. Runs the command returned by the supplier when this - command is initialized, and ends when it ends. Useful for performing runtime tasks before - creating a new command. If this command is interrupted, it will cancel the command. + Defers Command construction to runtime. Runs the command returned by a supplier when this command + is initialized, and ends when it ends. Useful for performing runtime tasks before creating a new + command. If this command is interrupted, it will cancel the command. Note that the supplier *must* create a new Command each call. For selecting one of a preallocated set of commands, use :class:`commands2.SelectCommand`. @@ -21,6 +21,12 @@ class DeferredCommand(Command): def __init__(self, supplier: Callable[[], Command], *requirements: Subsystem): """ + + Creates a new DeferredCommand that directly runs the supplied command when initialized, and + ends when it ends. Useful for lazily creating commands when the DeferredCommand is initialized, + such as if the supplied command depends on runtime state. The Supplier will be called + each time this command is initialized. The Supplier *must* create a new Command each call. + :param supplier: The command supplier :param requirements: The command requirements. """ diff --git a/commands2/proxycommand.py b/commands2/proxycommand.py index 759b3504..6475cfab 100644 --- a/commands2/proxycommand.py +++ b/commands2/proxycommand.py @@ -7,12 +7,18 @@ from .command import Command from .util import format_args_kwargs +import warnings class ProxyCommand(Command): """ - Schedules the given command when this command is initialized, and ends when it ends. Useful for - forking off from CommandGroups. If this command is interrupted, it will cancel the command. + Schedules a given command when this command is initialized and ends when it ends, but does not + directly run it. Use this for including a command in a composition without adding its + requirements, but only if you know what you are doing. If you are unsure, see + `the WPILib docs `_ + for a complete explanation of proxy semantics. Do not proxy a command from a subsystem already + required by the composition, or else the composition will cancel itself when the proxy is reached. + If this command is interrupted, it will cancel the command. """ _supplier: Callable[[], Command] @@ -21,9 +27,16 @@ class ProxyCommand(Command): def __init__(self, supplier: Callable[[], Command]): """ Creates a new ProxyCommand that schedules the supplied command when initialized, and ends when - it is no longer scheduled. Useful for lazily creating commands at runtime. + it is no longer scheduled. it is no longer scheduled. Use this for lazily creating **proxied** commands at + runtime. Proxying should only be done to escape from composition requirement semantics, so if + only initialization time command construction is needed, use DeferredCommand instead. :param supplier: the command supplier + This constructor's similarity to DeferredCommand is confusing and opens + potential footguns for users who do not fully understand the semantics and implications of + proxying, but who simply want runtime construction. Users who do know what they are doing + and need a supplier-constructed proxied command should instead proxy a DeferredCommand + using the ``asProxy`` decorator. """ ... @@ -43,6 +56,9 @@ def __init__(self, *args, **kwargs): def init_supplier(supplier: Callable[[], Command]): assert callable(supplier) self._supplier = supplier + warnings.warn( + "The ProxyCommand supplier constructor has been deprecated", DeprecationWarning, stacklevel=2 + ) def init_command(command: Command): self.setName(f"Proxy({command.getName()})") From ac171ee8abfb8615a1da6291be068817a4d82e08 Mon Sep 17 00:00:00 2001 From: DeltaDizzy Date: Sun, 28 Apr 2024 23:21:15 -0400 Subject: [PATCH 2/4] run black --- commands2/proxycommand.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/commands2/proxycommand.py b/commands2/proxycommand.py index 6475cfab..f216875b 100644 --- a/commands2/proxycommand.py +++ b/commands2/proxycommand.py @@ -14,10 +14,10 @@ class ProxyCommand(Command): """ Schedules a given command when this command is initialized and ends when it ends, but does not directly run it. Use this for including a command in a composition without adding its - requirements, but only if you know what you are doing. If you are unsure, see + requirements, but only if you know what you are doing. If you are unsure, see `the WPILib docs `_ - for a complete explanation of proxy semantics. Do not proxy a command from a subsystem already - required by the composition, or else the composition will cancel itself when the proxy is reached. + for a complete explanation of proxy semantics. Do not proxy a command from a subsystem already + required by the composition, or else the composition will cancel itself when the proxy is reached. If this command is interrupted, it will cancel the command. """ @@ -57,7 +57,9 @@ def init_supplier(supplier: Callable[[], Command]): assert callable(supplier) self._supplier = supplier warnings.warn( - "The ProxyCommand supplier constructor has been deprecated", DeprecationWarning, stacklevel=2 + "The ProxyCommand supplier constructor has been deprecated", + DeprecationWarning, + stacklevel=2, ) def init_command(command: Command): From af9ec028091e4f296785ae7833f2b37a018fe3b2 Mon Sep 17 00:00:00 2001 From: Vasista Vovveti Date: Sun, 28 Apr 2024 20:27:18 -0700 Subject: [PATCH 3/4] Update commands2/proxycommand.py --- commands2/proxycommand.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/commands2/proxycommand.py b/commands2/proxycommand.py index f216875b..7fff0e89 100644 --- a/commands2/proxycommand.py +++ b/commands2/proxycommand.py @@ -27,7 +27,7 @@ class ProxyCommand(Command): def __init__(self, supplier: Callable[[], Command]): """ Creates a new ProxyCommand that schedules the supplied command when initialized, and ends when - it is no longer scheduled. it is no longer scheduled. Use this for lazily creating **proxied** commands at + it is no longer scheduled. Use this for lazily creating **proxied** commands at runtime. Proxying should only be done to escape from composition requirement semantics, so if only initialization time command construction is needed, use DeferredCommand instead. From 92e053f5b1f14894b9c833db9b39d6fb3ec9dd3b Mon Sep 17 00:00:00 2001 From: DeltaDizzy Date: Mon, 29 Apr 2024 14:40:51 -0400 Subject: [PATCH 4/4] increase stacklevel --- commands2/proxycommand.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/commands2/proxycommand.py b/commands2/proxycommand.py index 7fff0e89..21d1cb14 100644 --- a/commands2/proxycommand.py +++ b/commands2/proxycommand.py @@ -59,7 +59,7 @@ def init_supplier(supplier: Callable[[], Command]): warnings.warn( "The ProxyCommand supplier constructor has been deprecated", DeprecationWarning, - stacklevel=2, + stacklevel=3, ) def init_command(command: Command):