wooey / clinto

This converts an assortment of python command line interfaces into a language agnostic build spec for usage in GUI creation.
BSD 3-Clause "New" or "Revised" License
17 stars 6 forks source link

Exception-catching monkey-patching approach to argparse extraction #16

Closed mfitzp closed 9 years ago

mfitzp commented 9 years ago

This approach passes a monkey-patched version of argparse to the executing script that raises an exception when it hits parse_args().

Scripts are able to execute normally up to that point meaning all kinds of imports and weird script constructions and other things should work just fine.

On hitting parse_args an exception is raised passing in the current argument parser. This is then extracted out of the exception back in clinto.

mfitzp commented 9 years ago

Give it a whirl, it works on every script I've tried here.

The code around this addition is getting ugly with the alternate approaches. Is this something that would be fixed in the generic parser work?

mfitzp commented 9 years ago

Ha. Just tried running Wooey alongside this and it breaks the Django commandline:

Traceback (most recent call last):
  File "manage.py", line 10, in <module>
    execute_from_command_line(sys.argv)
  File "/usr/local/lib/python2.7/site-packages/django/core/management/__init__.py", line 338, in execute_from_command_line
    utility.execute()
  File "/usr/local/lib/python2.7/site-packages/django/core/management/__init__.py", line 330, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/usr/local/lib/python2.7/site-packages/django/core/management/base.py", line 384, in run_from_argv
    options = parser.parse_args(argv[2:])
  File "/usr/local/lib/python2.7/site-packages/django/core/management/base.py", line 64, in parse_args
    return super(CommandParser, self).parse_args(args, namespace)
  File "/usr/local/lib/python2.7/site-packages/clinto/parser.py", line 120, in parse_args_monkeypatch
    raise WooeyArgumentParserException(self)
clinto.parser.WooeyArgumentParserException

I don't think there is a way to import multiple copies of a module in Python (a fact we're relying on to make this work). So I guess we have two options:

- use multiprocessing or similar to run this in a thread (that should force a second import) though we will also need to patch inside the thread: we can probably just stuff our monkeypatch code into the head of the script before execution
- monkeypatch on a custom function, e.g. `parse_args_clinto`, and search-replace for `.parse_args(` > `.parse_args_clinto(` in the source (this feels a bit more prone to error, though simpler)

Also realised I called the exception Wooey* when it should be Clinto* will fix!

mfitzp commented 9 years ago

I tried the multiprocessing approach and hit the issue that ArgumentParser objects can't be pickled because they have methods for some parameters (help formatter for example). But then it occurred to me that a simple workaround would be to monkey patch the module locally and unmonkey it after. This is what the latest commit does and it appears to work fine both standalone and with Wooey.

Testing this with the examples in #10 this appears to fix that bug also.

Looking at click it may be possible to use this same trick in https://github.com/mitsuhiko/click/blob/master/click/parser.py to catch the parse_args step and return the parser. Probably a lot easier than trying to figure out click internals: the parser backend is based on optparse.

Chris7 commented 9 years ago

That's a good file to reference for click! I took the approach of looking for subclasses of click. Because click operates via decorators, we can always look at the global namespace so it's actually a bit easier to get the click parser object.

mfitzp commented 9 years ago

These changes were merged in as part of #17