-
-
Notifications
You must be signed in to change notification settings - Fork 309
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
grass.script: Automatically parse JSON and CSV in parse_command #3687
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great update overall. The heuristic should work well the format parameter which in combination with the parse_command function will almost always refer to output text format.
A short change for what it accomplishes!
Thanks for changing to csv.DictReader, the purpose here is convenience, so DictReader seems more appropriate than plain reader.
I agree with converting the generator to a list. json.loads also returns plain objects, so it is consistent with that.
>>> type(json.loads("""{"a": 1}"""))
<class 'dict'>
>>> type(json.loads("""[1,2]"""))
<class 'list'>
The delimiter change is a good clean up.
Please, add tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the tests! They nicely show how powerful this is.
python/grass/script/testsuite/test_start_command_functions_nc.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome. Looking forward to using this. It will work great together with the JSON GSoC project.
Given several tools have options
format
, this adds automatic parsing toparse_command
:For csv, I use DictReader, which makes sense only when the csv has a header, but that seems like reasonable assumption, and then it works nicely with pandas.
Option delimiter does not fit well into it conceptually, so I deprecated it. Also, it's now taken into account only when parameter
parse
is not specified, it assumed that anyway.