t.info: add format option and JSON output support#7043
t.info: add format option and JSON output support#7043sakirr05 wants to merge 12 commits intoOSGeo:mainfrom
Conversation
ninsbl
left a comment
There was a problem hiding this comment.
Some things to consider:
- the implementation seems incomplete / too specific. Please search the codebase for example for print_shell or print_self to see relevant occurences.
- Did you consider using a custom JSONEncoder for datetime (instead of your private function)?
- Please have a look at the shell output of
t.info. JSON format has to reflect all elements there. - There is a parser standard option for formats: https://grass.osgeo.org/grass-devel/manuals/parser_standard_options.html#g_opt_f_format (search the code base for: G_OPT_F_FORMAT for inspiration)
- In the tests please also test that metadata is returned properly (right values) not only the presence of selected keys.
Consider the following (not an implementation!):
from datetime import datetime
from json import dumps, JSONEncoder
import grass.temporal as tgis
tgis.init()
# Create custom JSONEncoder (may go into grass.script ?!? with a different name)
class DateTimeEncoder(JSONEncoder):
# Override the default method
def default(self, obj):
if isinstance(obj, (date, datetime)):
return obj.isoformat(" ")
# Open an existing STRDS
stds = tgis.open_old_stds("A@PERMANENT", "strds")
# Extract metadata to dict
md = {**stds.base.__dict__.get("D"), **stds.spatial_extent.__dict__.get("D"), **stds.temporal_extent.__dict__.get("D"), **stds.metadata.__dict__.get("D")}
# Print in JSON format
print(dumps(md, indent=4, cls=DateTimeEncoder))
#compare output to output of
stds.print_shell_info()
And please, read again the AI-usage policy! Thought-through PRs are much prefered over super-fast PRs that rely quite a bit on AI generated code...
|
hey @ninsbl with existing print_shell / print_self patterns and revisit the JSON handling, including using a custom JSONEncoder for datetime as suggested. |
- Add TemporalJSONEncoder in abstract_dataset.py (datetime -> ISO via default()) - Use json.dumps(..., cls=TemporalJSONEncoder, indent=4); remove default= - Keep print_json() alongside print_info/print_shell_info; print_self separate - _to_json_dict() uses same getters as print_shell_info (no parsing)
- format option: key=format, options=plain,shell,json, default=plain - Descriptions match standard GRASS modules (Human readable, Shell script style, JSON) - guisection: Print; -g without explicit format -> shell (backward compatibility)
- Assert metadata values: name, mapset, id, temporal_type, extent, number_of_maps - Assert start_time/end_time in ISO format - Replace key-only checks with value assertions; use deterministic r.mapcalc expression
310fa56 to
61b5bab
Compare
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
ninsbl
left a comment
There was a problem hiding this comment.
Hei @sakirr05 . I have been waiting with a review, as I expected some more changes:
- use of the standard parser option (G_OPT_F_FORMAT) I mentioened earlier,that is still not used
- my comment about the PR being inncomplete / too specific may have been a bit confusing:
- With too specific I mean, the way you implemented printing of JSON now, would have to be updated for each metadata item that may get added or changed, while e.g. a dict-comprehension would not have to be touched in those cases.
- with incomplete I mean that when you add a method to an AbstractBaseClass, at least my understanding is, that all classes derived from that should implement that method, no? You just added an implementation to a subset... Also it seems you are still missing some metadata keys (e.g. min_min, min_max, ...).
- The TemporalDataEncoder looks OK, it may be even better to put it in an utils package, as it maybe useful beyond the temporal framework...
I hope others with a better understanding of design pattern in GRASS can chime in (and maybe also relax a bit my requests for changes)...
Replace manual key enumeration with D dict merging. Ensures shell/JSON parity and future-proof updates.
Enables Python scripts to use G_OPT_F_FORMAT via g.parser.
Adds _to_json_dict override for computed fields not in metadata.D.
c27e90e to
6c111d2
Compare
0adec5a to
61e0c7b
Compare
ninsbl
left a comment
There was a problem hiding this comment.
Much better! Thanks for addressing my earlier comments.
Sorry for not being clear: when I wrote that the JSON decoder class is probebly better served in an "utils" module I actually thought of somethin like grass.script.utils...
@wenzeslaus or @echoix would you agree here? probably even change the name to GRASSJsonEncoder (or similar?
Please also test other STDS types (STVDS, STR3DS) and check again that all keys from shell output are present in JSON...
|
Thanks @ninsbl For testing STVDS/STR3DS — do you want separate test functions for each, or |
I added a link to the existing file in my comment...
STVDS and STR3DS are different types of Space Time Datasets: https://grass.osgeo.org/grass-stable/manuals/temporalintro.html Note otherwise also: #7071 |
Move TemporalJSONEncoder from grass.temporal.utils to grass.script.utils for broader reusability. A re-export is kept in grass.temporal.utils for backward compatibility. Add test classes for STVDS (vector) and STR3DS (3D raster) dataset types with value-based metadata assertions and shell/JSON parity verification for all three STDS types.
|
@ninsbl I’ve pushed the updates, The encoder is now in grass.script.utils (with a re-export kept in grass.temporal.utils for compatibility). I'll rename it if there’s a preferred naming convention also added separate tests for STVDS and STR3DS, including metadata checks, type specific fields, and shell/JSON key parity to catch missing fields. |
ref #7041
Instead of serializing or parsing the existing text output, JSON support is implemented at the temporal dataset abstraction level and built directly from the dataset’s metadata via existing getter methods. This keeps the output stable, structured, and suitable for scripting.
Details:
Adds a format option to t.info (plain, shell, json; default: plain)
Introduces print_json() for temporal datasets
Existing plain, shell, and -g output remains unchanged
JSON output uses the standard Python json module
Pytest coverage added for JSON output
Example:
t.info` type=strds input=precip_abs1 format=jsonExample output:
Tests:
New pytest tests ensure that t.info format=json returns valid JSON and includes the expected core metadata. Tests require a GRASS session and are intended to run in CI.