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

Unexpected parsing behaviour for --date=2018-12-09 vs. --date=2018-12-10 #102

Closed
martinvirtel opened this issue Nov 28, 2017 · 4 comments
Closed

Comments

@martinvirtel
Copy link

@martinvirtel martinvirtel commented Nov 28, 2017

Anything that can be interpreted as an arithmetics expression will be magically "calculated" by fire. This is very practical. But it bit me because I wanted to parse dates from command line parameters using fire, see below.

Suggestion: accept type hints as a way to prevent this. A parameter with type hint "str" shoud not be "calculated".

Test code:


def test(string: str=None) :
    print(string)

if __name__=="__main__" :
    fire.Fire(test)

Will give you:

> python ./test-parsing.py --string=2010-10-10
1990    # not interpreted as a string, but as 2010 minus 10 minus 10

> python ./test-parsing.py --string=2010-10-09
2010-10-09

martinvirtel pushed a commit to martinvirtel/picturalliance.py that referenced this issue Nov 28, 2017
@dbieber
Copy link
Member

@dbieber dbieber commented Nov 28, 2017

Definitely agree that this is an issue.
In my view, it's also an issue even when there is no type annotation.
Expressions like 2010-10-10 should not be evaluated by Fire; only literals and collections of literals should be parsed by default. (Issue #97)

@danishprakash
Copy link

@danishprakash danishprakash commented Oct 9, 2018

I would like to fix this @dbieber if nobody else is and while we're at it, do you already have an approach in mind for tackling this?

@dbieber
Copy link
Member

@dbieber dbieber commented Oct 29, 2018

Yes, see my comment here: #97 (comment)

@dbieber
Copy link
Member

@dbieber dbieber commented Feb 28, 2020

I think this was fixed at least by 0.2.1, though maybe it was earlier.

@dbieber dbieber closed this Feb 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants