-
Notifications
You must be signed in to change notification settings - Fork 128
Getting TypeError on invoking command from 2.5.7 #1525
-
Newly added code arg_parser = cmd2_app._command_parsers.get(cmd_wrapper) getting TypeError in deepcopy:
Traceback (most recent call last):
File "python3.10/site-packages/cmd2/cmd2.py", line 2552, in onecmd_plus_hooks
stop = self.onecmd(statement, add_to_history=add_to_history)
File "python3.10/site-packages/cmd2/cmd2.py", line 3031, in onecmd
stop = func(statement)
File "python3.10/site-packages/cmd2/decorators.py", line 355, in cmd_wrapper
arg_parser = cmd2_app._command_parsers.get(cmd_wrapper)
File "python3.10/site-packages/cmd2/cmd2.py", line 263, in get
parser = self._cmd._build_parser(parent, parser_builder, command)
File "python3.10/site-packages/cmd2/cmd2.py", line 782, in _build_parser
parser = copy.deepcopy(parser_builder)
File "/auto/pysw/cel8x/python64/3.10.10/lib/python3.10/copy.py", line 172, in deepcopy
y = _reconstruct(x, memo, *rv)
File "/auto/pysw/cel8x/python64/3.10.10/lib/python3.10/copy.py", line 271, in _reconstruct
state = deepcopy(state, memo)
File "/auto/pysw/cel8x/python64/3.10.10/lib/python3.10/copy.py", line 146, in deepcopy
y = copier(x, memo)
File "/auto/pysw/cel8x/python64/3.10.10/lib/python3.10/copy.py", line 231, in _deepcopy_dict
y[deepcopy(key, memo)] = deepcopy(value, memo)
File "/auto/pysw/cel8x/python64/3.10.10/lib/python3.10/copy.py", line 146, in deepcopy
y = copier(x, memo)
File "/auto/pysw/cel8x/python64/3.10.10/lib/python3.10/copy.py", line 206, in _deepcopy_list
append(deepcopy(a, memo))
File "/auto/pysw/cel8x/python64/3.10.10/lib/python3.10/copy.py", line 172, in deepcopy
y = _reconstruct(x, memo, *rv)
File "/auto/pysw/cel8x/python64/3.10.10/lib/python3.10/copy.py", line 271, in _reconstruct
state = deepcopy(state, memo)
File "/auto/pysw/cel8x/python64/3.10.10/lib/python3.10/copy.py", line 146, in deepcopy
y = copier(x, memo)
File "/auto/pysw/cel8x/python64/3.10.10/lib/python3.10/copy.py", line 231, in _deepcopy_dict
y[deepcopy(key, memo)] = deepcopy(value, memo)
File "/auto/pysw/cel8x/python64/3.10.10/lib/python3.10/copy.py", line 172, in deepcopy
y = _reconstruct(x, memo, *rv)
File "/auto/pysw/cel8x/python64/3.10.10/lib/python3.10/copy.py", line 271, in _reconstruct
state = deepcopy(state, memo)
File "/auto/pysw/cel8x/python64/3.10.10/lib/python3.10/copy.py", line 146, in deepcopy
y = copier(x, memo)
File "/auto/pysw/cel8x/python64/3.10.10/lib/python3.10/copy.py", line 231, in _deepcopy_dict
y[deepcopy(key, memo)] = deepcopy(value, memo)
File "/auto/pysw/cel8x/python64/3.10.10/lib/python3.10/copy.py", line 146, in deepcopy
y = copier(x, memo)
File "/auto/pysw/cel8x/python64/3.10.10/lib/python3.10/copy.py", line 238, in _deepcopy_method
return type(x)(x.__func__, deepcopy(x.__self__, memo))
File "/auto/pysw/cel8x/python64/3.10.10/lib/python3.10/copy.py", line 172, in deepcopy
y = _reconstruct(x, memo, *rv)
File "/auto/pysw/cel8x/python64/3.10.10/lib/python3.10/copy.py", line 271, in _reconstruct
state = deepcopy(state, memo)
File "/auto/pysw/cel8x/python64/3.10.10/lib/python3.10/copy.py", line 146, in deepcopy
y = copier(x, memo)
File "/auto/pysw/cel8x/python64/3.10.10/lib/python3.10/copy.py", line 231, in _deepcopy_dict
y[deepcopy(key, memo)] = deepcopy(value, memo)
File "/auto/pysw/cel8x/python64/3.10.10/lib/python3.10/copy.py", line 172, in deepcopy
y = _reconstruct(x, memo, *rv)
File "/auto/pysw/cel8x/python64/3.10.10/lib/python3.10/copy.py", line 271, in _reconstruct
state = deepcopy(state, memo)
File "/auto/pysw/cel8x/python64/3.10.10/lib/python3.10/copy.py", line 146, in deepcopy
y = copier(x, memo)
File "/auto/pysw/cel8x/python64/3.10.10/lib/python3.10/copy.py", line 231, in _deepcopy_dict
y[deepcopy(key, memo)] = deepcopy(value, memo)
File "/auto/pysw/cel8x/python64/3.10.10/lib/python3.10/copy.py", line 172, in deepcopy
y = _reconstruct(x, memo, *rv)
File "/auto/pysw/cel8x/python64/3.10.10/lib/python3.10/copy.py", line 271, in _reconstruct
state = deepcopy(state, memo)
File "/auto/pysw/cel8x/python64/3.10.10/lib/python3.10/copy.py", line 146, in deepcopy
y = copier(x, memo)
File "/auto/pysw/cel8x/python64/3.10.10/lib/python3.10/copy.py", line 231, in _deepcopy_dict
y[deepcopy(key, memo)] = deepcopy(value, memo)
File "/auto/pysw/cel8x/python64/3.10.10/lib/python3.10/copy.py", line 172, in deepcopy
y = _reconstruct(x, memo, *rv)
File "/auto/pysw/cel8x/python64/3.10.10/lib/python3.10/copy.py", line 297, in _reconstruct
value = deepcopy(value, memo)
File "/auto/pysw/cel8x/python64/3.10.10/lib/python3.10/copy.py", line 172, in deepcopy
y = _reconstruct(x, memo, *rv)
File "/auto/pysw/cel8x/python64/3.10.10/lib/python3.10/copy.py", line 265, in _reconstruct
y = func(*args)
File "/auto/pysw/cel8x/python64/3.10.10/lib/python3.10/copyreg.py", line 101, in __newobj__
return cls.__new__(cls, *args)
TypeError: Device.__new__() missing 1 required positional argument: 'name'
Not sure why the Device object(custom class in my app) is getting deepcopied?
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.
All reactions
Parsers have been deep copied since cmd2 2.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 the Device object 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, Device was present in the parser, and it crashed trying to deep copy it.
Does this description sound correct for what you're seeing?
S...
Replies: 4 comments 9 replies
-
Parsers have been deep copied since cmd2 2.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 the Device object 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, Device was present in the parser, and it crashed trying to deep copy it.
Does this description sound correct for what you're seeing?
Suggested Fix
The easiest way to fix this is to edit the parser after it has been deep copied.
We deep copy parsers, so that each cmd2.Cmd instance has independent copies of them. In your case, I think you're adding a Device instance to a parser which is a class variable. If you ever needed to create another instance of your CLI class, its parser would be pointing at the Device instance of the other CLI. This may not ever apply in your situation, but that's what we're trying to prevent.
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.
All reactions
-
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:
class MyCommandSet(CommandSet):
def __init__(self, ...):
super().__init__()
my_ap = Cmd2ArgumentParser()
my_ap.add_argument('xxx', completer=self.completion_factory.get_completer(CompleterType.MY_COMMAND_XXX))
orig_method = getattr(self.__class__, 'do_my_command')
ap_wrapped = with_argparser(tb_ap)(orig_method).__get__(self, self.__class__)
setattr(self, 'do_my_command', ap_wrapped)
def do_my_command(self, args):
...
May i know if the completion_factory gets deepcopied in this case? If so, then i am into big trouble as i don't want that to happen at all. I just want to initialize the completion functions using a separate factory that has other dependencies injected, instead of adding custom functions in the command set itself. My autocompletion also gets the TypeError along with the command invoke as well. Any ways to solve this issue then?
Also, one doubt, why is the error hit after the command has been processed?
Beta Was this translation helpful? Give feedback.
All reactions
-
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 cmd2 makes a call like this.
self._cmd._command_parsers.get(func)
If _command_parsers.get() does not yet have a parser for this function, it creates a deep copy.
On 2.5.6, this deep copy was happening earlier, and I believe certain instance data didn't yet exist, so it succeeded.
On 2.5.7, your application is in more of a running state and when the deep copy occurs, that Device instance exists which cannot be deep copied.
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 Fix
The 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 CommandSet.on_registered(). Here is its docstring.
""" 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.
All reactions
-
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.
All reactions
-
Can you provide a stack trace?
Beta Was this translation helpful? Give feedback.
All reactions
-
Actually, this command in turn calls self.onecmd() for invoking another command to do some post processing.
Beta Was this translation helpful? Give feedback.
All reactions
-
@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.
All reactions
-
OK, then the issue is no longer with my_command.
Does another_cmd also have an argument parser with a Device buried within?
Beta Was this translation helpful? Give feedback.
All reactions
-
Got it now. Let me check & get back
Beta Was this translation helpful? Give feedback.
All reactions
-
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.