Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

How can I use default arguments in a method with python-fire? Possible bug. Feature request: Argument in fire.Fire to force using keyword args #207

Closed
securisec opened this issue Oct 29, 2019 · 11 comments

Comments

@securisec
Copy link

securisec commented Oct 29, 2019

I am having an issue with python-fire when a method has arguments with default values. Consider the following code:

import fire

class SomeClass(object):
    def __init__(self):
        self.a = ''
        self.b = ''

    def methoda(self):
        self.a = 'A'
        return self

    def methodb(self, x='B123'):
        self.b = self.a + x
        return self

    def __str__(self):
        return self.b

if __name__ == '__main__':
    s = SomeClass()
    s.methodb().methoda()
    print(s.b) # this prints B123 correctly

    fire.Fire(SomeClass)

As seen in the comments, in the print(s.b), it is printing B123` correctly. But when i change the order of the methods being called in fire from the command line, I am getting the odd behavior.

Example:

> python x.py
B123 # correct

> python x.py methoda methodb
Here: B123
AB123 # both correct

> python x.py methodb --x B123 methoda
Here: B123
B123 # again both correct

> python x.py methodb methoda
Here: B123
methoda # this is not correct. It should print B123 here also

As you can see with the last example, if i call methodb (which has an argument with a default value), it prints methoda instead of B123 as expected.

My question is, How can I use a method that has a default argument value first in this type of scenario without passing in --x=something?

In short, how can i make > python x.py methodb methoda properly print B123?
Second question is, how can I show the method args in the help? Currently it shows nothing when there is a default value assigned. When I do methoda methodb, I do not have to pass --x=B123, but if I move the method with the default argument value up, it doesnt work as seen in the example.

@securisec securisec changed the title How can I use default arguments in a method with python-fire? How can I use default arguments in a method with python-fire? Possible bug Oct 29, 2019
@dbieber
Copy link
Member

dbieber commented Oct 29, 2019

Thanks for the questions.

python x.py methodb methoda

Try instead:

python x.py methodb - methoda

By inserting a separator, you are telling Fire that you are done passing arguments to methodb.

how can I show the method args in the help?

I'll come back and look at this question at a later time.

@securisec
Copy link
Author

Yes, I agree with what you say about using the separator, but my confusion is in two cases:

  • the default argument value works fine without the seperator when methodb is at the end
  • when methodb is in the beginning, it doesn't seem to treat the default argument value.

Thanks for looking into this when you have a moment!

@securisec
Copy link
Author

@dbieber bump

@dbieber
Copy link
Member

dbieber commented Nov 1, 2019

When you do methodb methoda the string "methoda" is passed as an argument to methodb, whereas when you do methoda methodb methoda is called first with no arguments, because it takes no arguments, and then methodb is called on the result.

If you want to force use of keyword arguments rather than allowing the argument to methodb to be passed positionally, you can try:

fire.decorators._SetMetadata(SomeClass.methodb, fire.decorators.ACCEPTS_POSITIONAL_ARGS, False)

I haven't checked if this works and it's not guaranteed to be supported long term though.

@securisec
Copy link
Author

Thanks @dbieber , i will give this a shot. In your example, you are passing the method to the decorator, but i will try passing my class to it (there are too many methods). Hopefully it threats the defaults argument values correctly.

Maybe a feature request will be to have an optional argument in fire.Fire that will allow the user to force use of keyword arguments?

btw, would this patch be applied before or after calling fire.Fire?

@dbieber
Copy link
Member

dbieber commented Nov 1, 2019

btw, would this patch be applied before or after calling fire.Fire?

before

(It is during the call to fire.Fire that your users command is executed.)

Maybe a feature request will be to have an optional argument in fire.Fire that will allow the user to force use of keyword arguments?

thanks for the request, I agree that configuring Fire should be a better experience

@securisec securisec changed the title How can I use default arguments in a method with python-fire? Possible bug How can I use default arguments in a method with python-fire? Possible bug. Feature request: Argument in fire.Fire to force using keyword args Nov 1, 2019
@securisec
Copy link
Author

securisec commented Nov 1, 2019

@dbieber thanks!, I just tried your solution, and unfortunately, that didnt work.

edit:

Didnt work for the class, but works for the individual method.

for method in dir(SomeClass):
        if not method.startswith('_'):
            fire.decorators._SetMetadata(getattr(SomeClass, method), fire.decorators.ACCEPTS_POSITIONAL_ARGS, False)

This works, but it seems super ugly...

@securisec
Copy link
Author

securisec commented Nov 8, 2019

@dbieber this question is not related to this post, but I couldnt think of a better person to ask this.

I know if how I can access and override the command being passed to the Fire instance.

So for instance:

while True:
    commands = 'my --commands being --passed to the class'
    fire_obj = fire.Fire(MyClass, command=commands)

So now I can access an instance of MyClass using fire_obj. But what I am trying to do is in the next iteration of the while loop, I want to change what fire.command holds. How can I access that? Specifically I am trying to access a couple of things

  • Any exceptions (because my handler is pretty printing the message, so i want to change some vars based on an exception)
  • Change the instance in fire if need be

I wrote a short post about how to combine using fire and prompt_toolkit for awesomeness.

Thanks!

@bmccutchon
Copy link

Re the feature request: IIUC, you can just do this in Python 3:

def methodb(self, *, x='B123'):

This seems to work, though --x will be marked as required in the documentation due to #199, even though it's not actually required.

@dbieber
Copy link
Member

dbieber commented Nov 15, 2019

@bmccutchon Thanks for sharing that 👍. Yes, using that approach will make x a keyword only argument which will mean it can only be passed as a flag, not positionally. So methoda will not be consumed as a positional argument to x.

@securisec I'm not sure I understand what you're trying to do in the loop. It sounds like you can just pass a different value to commands in each iteration of the loop, however, I don't know why you want to call Fire repeatedly in a loop to begin with. But if you want to continue this discussion, let's do it in a brand new issue so we keep each issue to a single topic. (And cool post :), thanks for sharing.)

@securisec
Copy link
Author

Thanks! although @bmccutchon is very elegant, and it works, it has two issues.

  • This would require significant rework on existing code
  • Still does not address the issue of default argument value.

At the moment, the response on #207 (comment) seems to be a much more effective solution. Ideally, a force keywords would be a much better solution.

@dbieber I figured out the solution to my other question. I will create a new issue with what I cant figure out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants