-
-
Notifications
You must be signed in to change notification settings - Fork 327
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
r.info: Add JSON output #3744
r.info: Add JSON output #3744
Conversation
Use parson to add json output format support to the r.info module. The module has various flags to control the fields being output in case of plain format. The current prototype adheres to the flags for JSON output as well.
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
I'm curious what others think about making parts of the JSON dependent on the flags versus always outputting everything. Compared to our previous discussion, I see that respecting flags is actually easier than not respecting them given the current code. On the other hand, adding |
I think we should start with outputting everything. @kritibirda26 will you add the JSON schema to your original PR comment. |
@cwhite911 Makes sense, I will update the code to output everything. By JSON schema, do you mean a valid https://json-schema.org/ or just a rough outline of the intended JSON output? |
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Does JSON output actually solve: #491 ? |
I agree the text returned as json should not contain line breaks. @kritibirda26 |
@cwhite911 AFAIK, it is not possible to set integers selectively using parson. My understanding is that all numbers should be integers or floats. See kgabis/parson#71 |
Strings should probably be escaped |
@kritibirda26 Will you look into some possible solutions. |
@cwhite911 I had looked into finding a solution before but couldn't fine one. The best I could come up with is setting static int format_json_number(double num, char* buf) {
long truncated = (long) num;
if (truncated == num) {
return sprintf(buf, "%ld", truncated);
} else {
return sprintf(buf, "%lf", num);
}
} We still cannot control individual number field's datatypes but it will format numbers as integers if possible. (The downside is some floating point fields might be formatted as numbers if their values are integers without any fractional part). Can you please tell if this solution is fine or if you have a better solution in mind? |
I don't have any suggestion at this point, but I remember I struggled with this when reading JSON into Python, sometimes the values for the same key came as ints and sometimes as floats depending on how they are stored in JSON. Considering the following example in Python: >>> type(json.loads('{"a": 7}')["a"])
<class 'int'>
>>> type(json.loads('{"a": 7.1}')["a"])
<class 'float'> This is expected, but it depends on the tool which produces the JSON whether or not it makes the distinction and would export 7.0 as 7.0 or 7. In Python, mixing floats and ints gives the expected float results and even division of ints now gives float, so it is less of an issue. If one or the other type is needed consistently, the only two ways I know about (without invoking things JSON schema reading) are knowing what type that should be (you know that for resolution you should use float) or exporting the types in JSON (makes more sense in cases when these are dynamic as in v.db.select). Just for comparison, Python JSON dump is preserving float vs int: >>> import json
>>> json.dumps({"a": 7})
'{"a": 7}'
>>> json.dumps({"a": 7.0})
'{"a": 7.0}'
>>> json.dumps({"a": int(7)})
'{"a": 7}'
>>> json.dumps({"a": float(7)})
'{"a": 7.0}'
>>> json.dumps({"a": 7.000})
'{"a": 7.0}' |
All versus selected:
|
So by reduction, the |
@kritibirda26 @cwhite911 Do you want to reevaluate the enabled flags? Perhaps it should give the same output as |
@wenzeslaus Can you please tell what we is to be reevaluated again and what the aim is? |
Given the [discussion](#3744 (comment)) about what should be part of JSON output, I changed the behavior to include the distance whether or not the -l flag is used. There is no computational reason not to.
There is
The Not now, but for version 9, we may want to do:
|
@wenzeslaus I agree, let's have the JSON format respect the '-s' flag. |
Hi! I have updated the PR to respect the |
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. Let's merge it. The code looks good and the tests, too.
We can always get back to this. (We have the branch for the 8.4 release series, so changes on main can be adjusted even in terms of API until branching for 8.5.)
Given the [discussion](OSGeo#3744 (comment)) about what should be part of JSON output, I changed the behavior to include the distance whether or not the -l flag is used. There is no computational reason not to.
Uses parson to add JSON output format support to the r.info module. The tool has various flags to control the fields being output in case of the shell script style format (aka plain in the code). The JSON output always contains all fields even without the flags being specified, except for statistics. The statistics must be set with a flag because the statistics are only optionally stored in the raster data and computing them on the fly takes a lot of time. The option value for format is plain (shell/key-value) and json and internally, the plain, human output is treated separately from the machine readable formats which are PLAIN and JSON.
Use parson to add json output format support to the r.info module. The module has various flags to control the fields being output in case of plain format. The current prototype adheres to the flags for JSON output as well.