Comments (7)
Yeah, that may make more sense.
One option that preserves backwards compatibility is
def Fire(component, command=None, name=None):
name = name or os.path.basename(sys.argv[0]) # (Note: Currently we don't set name from sys.argv[0] if command is set, and I think that's a bug.)
if isinstance(command, six.string_types):
args = shlex.split(command)
elif isinstance(command, (list, tuple)):
args = command
elif command is None:
args = sys.argv[1:]
else:
raise ValueError()
I would rather "get it right" than preserve backwards compatibility since the project is young and these are undocumented and largely unused arguments.
I'm tempted to (once completion is updated) remove the name
argument altogether and always use the default of os.path.basename(sys.argv[0])
. What do you think of this? (e.g. is it a problem people are using aliases? what are the use cases where the CLI name should differ from sys.argv[0]? How should name
be set when command
/args
is supplied?)
We should also try to get the argument names right:
argparse uses the argument prog
whereas we currently use name
.
For the arguments/command, we could do command
, args
, argv
. To me, command sounds like a string, args sounds like the list sys.argv[1:], and argv sounds like the list sys.argv.
I think my preference is:
def Fire(component, args=None, name=None):
name = name or os.path.basename(sys.argv[0])
if isinstance(args, six.string_types):
args = shlex.split(args)
elif args is None:
args = sys.argv[1:]
if not isinstance(args, (list, tuple)):
raise ValueError()
I think it's ok that this renames command
and therefore isn't backwards compatible w/ that undocumented argument. I also think it's OK that args can accept a string as well as a list. Using the name args over command encourages people to pass a list, which is the better/safer way to use this, though passing a string is the 'easier' way.
One concern here is that if we later remove the name
argument, that leaves no way to change the name used in the usage strings short of modifying sys.argv (which I definitely don't want to encourage). e.g. you can't pass in an args list w/ args[0] set differently. Maybe I'm wrong to want to remove the name
argument :).
from python-fire.
just a thought - I think it'd be more consistent to accept the equivalent of argv[1:]
e.g. here's snippet from argparse.py:
if args is None:
args = _sys.argv[1:]
https://svn.python.org/projects/python/trunk/Lib/argparse.py
from python-fire.
I agree that breaking backwards compatibility is the right choice here - especially if you can carry over knowledge from argparse to fire.
My bikeshed:
- agree on
args
overcommand
. name
vs.prog
- eh - click doesn't even allow you to setname
, butname
is possibly more straightforward thanprog
. /shrug
I believe the reason you'd want to overwrite name is when aliasing commands (like gvim
/ vim
/ vimdiff
/ etc). But agreed I'm not sure why you'd overwrite it.
from python-fire.
I think I agree with both of you there. Make a clean interface, via args
rather than command
. I'm not sure how strong the use case is for name
, but there may well be one around usage strings as you note. But I recognize there are lots of subtleties here, and I'm not up-to-speed with all of the issues.
from python-fire.
For some context, right now we use name
in two places:
And there are two times I can think of where you'd maybe (?) want to set name
explicitly:
- If the command is commonly called by an alias
- If the command is commonly called inside of another script w/ a different name (if you want usage/completion to work with that outer script).
It's worth noting here that it's possible for a completion script to do alias resolution, though we don't do it yet.
I also need to think more about what name + usage strings should look like if you call Fire from inside a function that was already called by Fire (as we will likely do in #29).
from python-fire.
Having thought more about this, I'm inclined to keep the "command" argument rather than replacing it with "args". We'll make it accept both a list of args or a string. It does after all represent the Fire command, and while args would also be a suitable (perhaps better!) name, I've come to think that the backwards compatibility argument outweighs this. [I don't want to come under fire for breaking the API!]
[Relatedly, there is also precedent for accepting a command both as a string or a list of arguments (subprocess.py), though they use the name args.]
I also need to think more about what name + usage strings should look like if you call Fire from inside a function that was already called by Fire (as we will likely do in #29).
If you call Fire with:
- no command and no name -> command=sys.argv[1:], name=sys.argv[0]
- no command and a name -> command=sys.argv[1:], name=name
- command and no name -> command=command, name=sys.argv[0] or None(?)
- command and name -> command=command, name=name
The third case (command is supplied but name is not) is the most interesting one. I'm inclined to use sys.argv[0] in this case, but am also considering removing the name from the usage strings altogether in this case.
When calling Fire from a method launched with Fire, one will either:
a) want access to the original unparsed arguments,
b) want Fire to not further parse the arguments.
We unofficially enable a) with the decorator in decorators.py @SetParseFn(str)
, but we'll want to make it official for this to be useful. We don't support b.
from python-fire.
Resolved with b5053ba.
from python-fire.
Related Issues (20)
- [feature request] Exclude function (kw)args from synopsis, arguments and flags in help output
- guide sample code is entered incorrectly HOT 2
- Version flag alongside other commands HOT 3
- Remove test requirement on mock HOT 4
- Is it possible to pass arg via code and kwargs by cli (sys.argv) ? HOT 1
- cli for function created on the fly HOT 1
- ERROR: Could not consume arg: >> HOT 1
- AttributeError: 'module' object has no attribute 'PY34' HOT 1
- How to set the number of args at least one? HOT 1
- Python 2.7 no longer available in GitHub Actions by default HOT 1
- Unexpected printing (+paging) when using inspect HOT 3
- Cannot parse list of strings containing `is` HOT 5
- Gracias
- unable to install fire HOT 4
- Android 14 HOT 1
- What's the meaning of "available commands: as_interger_ratio | bit_count |..." HOT 2
- How to use the avialable command "S.xx", such as S.count HOT 2
- How to pass '2e9672320848' as a str value? HOT 1
- Optional type args are mistyped in help HOT 1
- Have all staff been laid off? HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from python-fire.