Skip to content

t.info: add format option and JSON output support - #7043

Open
sakirr05 wants to merge 510 commits into
OSGeo:mainfrom
sakirr05:tinfo-json-format
Open

sakirr05 wants to merge 510 commits into
OSGeo:mainfrom
sakirr05:tinfo-json-format

Conversation

@sakirr05

@sakirr05 sakirr05 commented Feb 5, 2026 •

Copy link
Copy Markdown
Contributor

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=json

Example output:

{
  "id": "precip_abs1@PERMANENT",
  "name": "precip_abs1",
  "mapset": "PERMANENT",
  "temporal_type": "absolute",
  "start_time": "2001-01-01T00:00:00",
  "end_time": "2012-12-31T00:00:00",
  "north": 228500,
  "south": 215000,
  "east": 645000,
  "west": 630000
}

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.

@github-actions github-actions Bot added temporal Related to temporal data processing Python Related code is in Python libraries module tests Related to Test Suite labels Feb 5, 2026

@ninsbl ninsbl left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some things to consider:

  1. the implementation seems incomplete / too specific. Please search the codebase for example for print_shell or print_self to see relevant occurences.
  2. Did you consider using a custom JSONEncoder for datetime (instead of your private function)?
  3. Please have a look at the shell output of t.info. JSON format has to reflect all elements there.
  4. 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)
  5. 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...

@sakirr05

sakirr05 commented Feb 5, 2026

Copy link
Copy Markdown
Contributor Author

hey @ninsbl with existing print_shell / print_self patterns and revisit the JSON handling, including using a custom JSONEncoder for datetime as suggested.
I’ll also review the shell output of t.info to ensure the JSON format reflects all relevant elements, and update the tests to verify correct metadata values, not just key presence and also check the parser standard format option (G_OPT_F_FORMAT) and see how best to integrate it consistently.
I’ll push an updated version addressing these points

@sakirr05
sakirr05 requested a review from ninsbl February 7, 2026 19:58

@ninsbl ninsbl left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hei @sakirr05 . I have been waiting with a review, as I expected some more changes:

  1. use of the standard parser option (G_OPT_F_FORMAT) I mentioened earlier,that is still not used
  2. 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, ...).
  1. 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)...

@github-actions github-actions Bot added C Related code is in C general labels Feb 10, 2026

@ninsbl ninsbl left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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...

@sakirr05

Copy link
Copy Markdown
Contributor Author

Thanks @ninsbl
Quick questions before I make the changes: Where should the encoder go exactly? I see you mentioned grass.script.utils but I'm not sure if that's an existing file I should add to or if you want a new module. Also should temporal modules still be able to import it directly?

For testing STVDS/STR3DS — do you want separate test functions for each, or
should I just extend the existing STRDS test to loop through all three types? And should I look at the linting failure first, or is that unrelated to these changes?
I'll hold off on the encoder name until @wenzeslaus or @echoix chime in.

@ninsbl

ninsbl commented Feb 11, 2026

Copy link
Copy Markdown
Member

I see you mentioned grass.script.utils but I'm not sure if that's an existing file I should add to or if you want a new module. Also should temporal modules still be able to import it directly?

I added a link to the existing file in my comment...

For testing STVDS/STR3DS — do you want separate test functions for each, or should I just extend the existing STRDS test to loop through all three types?

STVDS and STR3DS are different types of Space Time Datasets: https://grass.osgeo.org/grass-stable/manuals/temporalintro.html
In tests, you will have to create a dataset for each type and then test t.info on them...

Note otherwise also: #7071

@sakirr05

sakirr05 commented Feb 11, 2026 •

Copy link
Copy Markdown
Contributor Author

@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.
The G_OPT_F_FORMAT change from #7071 is already in this branch (commit b30ae7d)

@sakirr05
sakirr05 requested a review from ninsbl February 12, 2026 18:02
@ninsbl

ninsbl commented Feb 16, 2026

Copy link
Copy Markdown
Member

Hi @sakirr05 , before I invest more effort into reviewing this PR, could yhou please confirm that you read the AI-policy of the GRASS community and that your PR and commits in it adhere to esp. the Human-in-the-loop requirement.
In that context I am for example wondering what compatibility the re-export in grass.temporal.utils is meant to preserve? Can you explain your line of thinking here?

@sakirr05

Copy link
Copy Markdown
Contributor Author

@ninsbl Yes, I’ve read the GRASS AI Policy, I used Claude mainly for brainstorming a development plan and writing some test scaffolding. Regarding the re-export in grass.temporal.utils: it was meant to preserve compatibility in case something depended on that import path, but after your question and reviewing the context, I agree it’s not necessary and I’ll remove it in the next update.

Comment thread general/g.parser/standard_option.c Resolved
Comment thread python/grass/temporal/utils.py Resolved
Comment thread python/grass/temporal/utils.py Resolved
@ninsbl

ninsbl commented Jun 7, 2026

Copy link
Copy Markdown
Member

Sorry for hijacking your PR, @sakirr05
Modifying it directly seemed most efficient to me. I hope you do not mind. Documentation and tests need still some final polishing...

@github-actions github-actions Bot added docs markdown Related to markdown, markdown files labels Jun 8, 2026
@ninsbl

ninsbl commented Jun 8, 2026

Copy link
Copy Markdown
Member

Given that pytests for stvds, strds, and str3ds could be structured quite similar, I am considering consolidating pytests into just one, parametrized test. But that may get use close to the 300s timeout (not sure how timeout or possibly parallel runs would be handled) with one test file applied to different sessions (stvds, strds, and str3ds)...
Also with more pytests being written for temporal modules, would we be able to get runtimes down if more tests share the same fixures?

Comment thread temporal/t.info/t.info.py Resolved
@sakirr05

Copy link
Copy Markdown
Contributor Author

Thanks @ninsbl. Parametrizing over the three dataset types makes sense since the test logic is identical keeping the three session fixtures separate avoids state conflicts and still allows parallel runs with pytest-xdist i can restructure into one parametrized file before merging if that's the preferred direction.

On shared fixtures: factoring setup into a conftest.py at the temporal/ level would definitely help reduce startup overhead as more temporal tests get added

@echoix

echoix commented Jun 10, 2026

Copy link
Copy Markdown
Member

On shared fixtures: factoring setup into a conftest.py at the temporal/ level would definitely help reduce startup overhead as more temporal tests get added

You can do that of course. We just can’t have a conftest at the root of the repo, as long as we have an autotools-configure--based build; it would get deleted when configuring.

@github-actions github-actions Bot added windows Microsoft Windows specific GUI wxGUI related macOS macOS specific docker Docker related CI Continuous integration vector Related to vector data processing raster Related to raster data processing C++ Related code is in C++ HTML Related code is in HTML translation Message translation related database Related to database management display imagery raster3d notebook misc CMake nix labels Nov 1, 2026
@ninsbl

ninsbl commented Nov 1, 2026 •

Copy link
Copy Markdown
Member

Uff... Sorry, now I completely messed up the git history...

What is the simplest fix: opening a new, clean PR with the relevnt changes while tagging all involved authors?

try:
tree = ET.fromstring(fo.read())
tree = ET.fromstring(fobj.read())
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C Related code is in C C++ Related code is in C++ CI Continuous integration CMake database Related to database management display docker Docker related docs general GUI wxGUI related HTML Related code is in HTML imagery libraries macOS macOS specific markdown Related to markdown, markdown files misc module nix notebook Python Related code is in Python raster Related to raster data processing raster3d temporal Related to temporal data processing tests Related to Test Suite translation Message translation related vector Related to vector data processing windows Microsoft Windows specific

Projects

None yet

Development

Successfully merging this pull request may close these issues.