Getting TypeError on invoking command from 2.5.7 #1525
-
|
Newly added code Not sure why the Can i get some context please? I can provide more details on the command itself. It's working without any issues till 2.5.6. Seems like this is the PR #1384 that has created the issue. |
Beta Was this translation helpful? Give feedback.
Replies: 4 comments 9 replies
-
|
Parsers have been deep copied since The context behind this change goes back to issue #1002. I believe the reason it doesn't crash in 2.5.6 is the timing of when deep copies are made. Prior to 2.5.7, parsers were copied in On 2.5.7, we make the deep copy when the parser is first accessed via Does this description sound correct for what you're seeing? Suggested FixThe easiest way to fix this is to edit the parser after it has been deep copied. We deep copy parsers, so that each So going forward, consider the parsers defined at the class level as templates. Don't edit them directly. Instead edit their instance-specific copies like this: parser = self._command_parsers.get(self.do_my_command)
parser.device = my_device |
Beta Was this translation helpful? Give feedback.
-
|
Actually, i am setting completer functions using a factory that contains most of my custom helper classes which has instance variables that are not meant to be deepcopied. I initialize the command set this way: May i know if the Also, one doubt, why is the error hit after the command has been processed? |
Beta Was this translation helpful? Give feedback.
-
|
The error is occurring the first time the parser is needed. In your case, that's either when you try to run it or when you tab complete it. That's when self._cmd._command_parsers.get(func)If On 2.5.6, this deep copy was happening earlier, and I believe certain instance data didn't yet exist, so it succeeded. Either way, you don't want to deep copy instance data in an argument parser, so it doesn't really matter when we do it. Suggested FixThe way to fix this is by not storing anything in the parser which you don't want deep copied. Mutable data is an example of this. Add all your instance data to the parser in """
2nd step to registering, called by cmd2.Cmd after a CommandSet is registered and all its commands have been added.
Subclasses can override this to perform custom steps related to the newly added commands.
"""Here is a working example which illustrates how to do this. import argparse
from cmd2 import (
Cmd,
Cmd2ArgumentParser,
CommandSet,
with_argparser,
)
class MyCommandSet(CommandSet):
def __init__(self) -> None:
super().__init__()
# The data we don't want deep copied.
self.instance_choices = ["some", "choices"]
def on_registered(self) -> None:
"""
Called by cmd2.Cmd after a CommandSet is registered and all
its commands have been added.
"""
# Obtain your instance-specific parser. This is the
# deep copy, so it is safe to add instance data to it.
parser = self._cmd._command_parsers.get(self.do_my_command)
# Add an argument which uses instance data.
# This is where you would add the argument with the completer factory.
parser.add_argument(
"unsafe_to_deep_copy",
help="I use instance data.",
choices=self.instance_choices,
)
@staticmethod
def _build_my_command_parser() -> Cmd2ArgumentParser:
"""
A parser creating factory method.
I prefer factory methods to create my parsers. I find it cleaner,
but you can create your parser however you want.
Parser factories can also be standalone functions outside of the class
and classmethods.
@with_argparser accepts parser objects and factory methods.
"""
# Create the base parser for my_command.
# We will add instance data to its deep copy in on_registered()
parser = Cmd2ArgumentParser(description="Run my_command")
parser.add_argument("-s", "--safe", help="I need no instance data")
return parser
@with_argparser(_build_my_command_parser)
def do_my_command(self, args: argparse.Namespace) -> None:
self._cmd.poutput("We didn't crash!")
self._cmd.poutput("I will now edit state data by adding a choice.")
self.instance_choices.append("new_choice")
if __name__ == '__main__':
command_sets = [MyCommandSet()]
app = Cmd(command_sets=command_sets)
app.cmdloop() |
Beta Was this translation helpful? Give feedback.
-
|
Actually, i moved to what you had mentioned but still seeing the issue: class MyCommandSet(CommandSet):
def __init__(self, completion_factory, dep1, dep2, ...):
super().__init__()
# I store my dependencies here first
self.completion_factory = completion_factory
self.dep1 = dep1
self.dep2 = dep2
def on_registered(self) -> None:
"""
Moved all my argument adding code to here now instead of __init__
"""
my_ap = self._cmd._command_parsers.get(self.do_my_command)
my_ap.add_argument('xxx', completer=self.completion_factory.get_completer(CompleterType.MY_COMMAND_XXX))
@with_argparser(Cmd2ArgumentParser(description='My command'))
def do_my_command(self, args):
...I register this command set in the following way: class MyApp(cmd2.Cmd):
def __init__(self):
super().__init__(...)
# Create dependent classes
dep1 = Dep1()
dep2 = Dep2()
dep3 = Dep3()
completion_factory = CompletionFactory(dep3, ...)
# Create command sets
cmd_sets = [
MyCommandSet(completion_factory, dep1, dep2),
...
]
for s in cmd_sets:
self.register_command_set(s)
def main():
myapp = MyApp()
myapp.cmdloop()Can you please help me figure out if what i had done is correct in the way you had mentioned? Or is it something i am missing here? |
Beta Was this translation helpful? Give feedback.
-
|
Can you provide a stack trace? |
Beta Was this translation helpful? Give feedback.
-
|
Actually, this command in turn calls self.onecmd() for invoking another command to do some post processing. |
Beta Was this translation helpful? Give feedback.
-
@with_argparser(Cmd2ArgumentParser(description='My command'))
def do_my_command(self, args):
...
res = <some_processing>
self._cmd.onecmd_plus_hooks(f"another_cmd <res>") |
Beta Was this translation helpful? Give feedback.
-
|
OK, then the issue is no longer with |
Beta Was this translation helpful? Give feedback.
-
|
Got it now. Let me check & get back |
Beta Was this translation helpful? Give feedback.
-
|
Thanks a lot, Kevin. It works now. Really appreciate your help in going through the steps with me. Actually, i am in the midst of release & this caught up late after moving to latest from 2.0.0. |
Beta Was this translation helpful? Give feedback.
Parsers have been deep copied since
cmd22.5.0, but unfortunately this change is not mentioned in our CHANGELOG.The context behind this change goes back to issue #1002.
I believe the reason it doesn't crash in 2.5.6 is the timing of when deep copies are made. Prior to 2.5.7, parsers were copied in
cmd2.Cmd.__init__(). I assume you are adding theDeviceobject to your parser after this method runs, so it wasn't present when the parser was deep copied.On 2.5.7, we make the deep copy when the parser is first accessed via
self._command_parsers. At this point,Devicewas present in the parser, and it crashed trying to deep copy it.Does this description sound correct for what you're seeing?
S…