Skip to content

t.list: Add support for listing multiple dataset types - #7731

Open
saket0187 wants to merge 6 commits into
OSGeo:mainfrom
saket0187:Add-multiple-types-input-option-t.list
Open

t.list: Add support for listing multiple dataset types#7731
saket0187 wants to merge 6 commits into
OSGeo:mainfrom
saket0187:Add-multiple-types-input-option-t.list

Conversation

@saket0187

Copy link
Copy Markdown
Contributor

This PR implements the feature requested in #7695 to support listing multiple dataset types simultaneously, along with improvements in t.list tests.

@ninsbl @petrasovaa

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

Looks in general good to me. I have only minor comments inline below...

Comment thread temporal/t.list/t.list.py Outdated
Comment thread python/grass/temporal/list_stds.py Outdated
Comment thread temporal/t.list/t.list.md Outdated
Comment thread temporal/t.list/t.list.py Outdated
@ninsbl

ninsbl commented Jul 16, 2026

Copy link
Copy Markdown
Member

@petrasovaa it would be good if you could give your opinion on how JSON output should be handled / look like. Where should mapset or type go for example and should the json structure be consistent for single an multiple requested types I probably thonk yes...

Comment thread python/grass/temporal/list_stds.py Outdated
Comment thread temporal/t.list/t.list.py Outdated
Comment thread temporal/t.list/t.list.py Outdated
@petrasovaa

Copy link
Copy Markdown
Contributor

@petrasovaa it would be good if you could give your opinion on how JSON output should be handled / look like. Where should mapset or type go for example and should the json structure be consistent for single an multiple requested types I probably thonk yes...

So, to avoid different sets of keys in each item in JSON (raster vs strsds), how about allowing user to just combine the same type (e.g. strds, stvds) and not allow combining e.g. raster and strds. I would like to keep the flat structure of the JSON.

@saket0187
saket0187 requested review from ninsbl and petrasovaa July 17, 2026 14:02
Comment thread python/grass/temporal/list_stds.py Outdated
@github-actions github-actions Bot added temporal Related to temporal data processing Python Related code is in Python libraries module docs markdown Related to markdown, markdown files tests Related to Test Suite labels Jul 17, 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.

I tried to think through the PR on a bus once more. Sorry for bringing up some new stuff. I have no strong opinion there, but I would be glad if you could consider my comments and address them as you see fit.

Comment thread python/grass/temporal/list_stds.py Outdated
Comment thread python/grass/temporal/list_stds.py Outdated
Comment thread python/grass/temporal/list_stds.py Outdated
Comment thread python/grass/temporal/list_stds.py Outdated
Comment thread python/grass/temporal/list_stds.py Outdated
for dtype in type:
for mapset in dbif.tgis_mapsets:
if temporal_type == "absolute":
table = dtype + "_view_abs_time"

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.

If the intention is to be able to list all STDS in one go, should we not support also multiple / list input for temporal_type?
Sorry for bringing that up somewhat late...

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I agree and t.list already allows that, just not the function.

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.

That probably means the temporal_type should always go into the JSON and CSV output too, no?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The temporal_type is already in CSV and JSON with columns=all, it could be added explicitly as one of the options, but I am not sure it's worth it.

Comment thread python/grass/temporal/list_stds.py Outdated
for row in rows:
row_dict = dict(row)
if len(type) > 1:
row_dict["type"] = dtype

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.

Back to the question if the keys should be consistent, regardless if t.list is run with multiple or single type (or possibly single or multiple temporal_type input? I tend to say: make the dict (JSON) output structure consistent / predictable. But no strong opinion. It should just be deliberate.

Also, could this block be solved simpler with a dict-comprehension and/or a dict-update? (consult ruff --preview --select ALL)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The JSON should not have mixed items (each item in an array should have the same set of keys), which is the case now because we don't mix maps and datasets. In the comment above I was thinking to add the type key always for JSON and CSV. I hope I am not missing anything here.

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.

Right, but e.g semantic_label is only available for STRDS. So, should columns that do not exist for every dataset type just be empty? Just me thinking out loud...

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I commented on this below, I think we need to select the intersection of the different columns.

@saket0187
saket0187 requested a review from ninsbl July 20, 2026 14:35
@saket0187

Copy link
Copy Markdown
Contributor Author

I've tried to cover all the suggested changes:

  • Updated the docstrings.
  • Added support for multiple temporal types in get_dataset_list (kept the same style, but it now accepts a list for more than one temporal_type, Maybe we can allow comma-separated strings for this and split them? Since there are only two, I think?).
  • The type column is now always included for csv and json outputs, as suggested by @petrasovaa (Note: this doesn't apply to the line format. Even if the user specifies only columns=id, the output will automatically be id,type).
  • Used stds_type as the var name in get_dataset_list, as suggested by @ninsbl .

I hope I haven't missed anything! Also, apologies for the late response, I was a bit occupied with other things.

@saket0187
saket0187 requested a review from petrasovaa July 20, 2026 16:27

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

In other, similar tools, JSON output defaults to all columns in the output, if I remember correctly?!?
How is this handled here? Different types can have different columns...

@saket0187

saket0187 commented Jul 25, 2026

Copy link
Copy Markdown
Contributor Author

In other, similar tools, JSON output defaults to all columns in the output, if I remember correctly?!? How is this handled here? Different types can have different columns...

Yes, In t.list, if the format is JSON and no columns are specified, it defaults to columns="all".

Here is an example output of t.list type=strds,stvds format=json.

[
    {
        "id": "nc_lst_daily@PERMANENT",
        "name": "nc_lst_daily",
        "mapset": "PERMANENT",
        "temporal_type": "absolute",
        "semantic_type": "mean",
        "creation_time": "2026-06-03 18:15:06",
        "modification_time": "2026-06-03 18:18:05",
        "creator": "saket",
        "start_time": "2002-01-01 00:00:00",
        "end_time": "2004-10-05 00:00:00",
        "granularity": "1 day",
        "north": 279073.975466,
        "south": 113673.975466,
        "east": 798143.311797,
        "west": 595143.311797,
        "bottom": 0.0,
        "top": 0.0,
        "proj": "XY",
        "raster_register": "raster_map_register_9ca4f56710e5453291c0c830aa8f9a92",
        "number_of_maps": 1008,
        "nsres_min": 200.0,
        "ewres_min": 200.0,
        "nsres_max": 200.0,
        "ewres_max": 200.0,
        "min_min": -1.49,
        "min_max": 30.63,
        "max_min": -0.29,
        "max_max": 48.23,
        "aggregation_type": null,
        "title": "NC Daily LST",
        "description": "Standard MODIS LST Time Series for North Carolina",
        "command": "...",
        "type": "strds"
    },
    {
        "id": "schools_stds@PERMANENT",
        "name": "schools_stds",
        "mapset": "PERMANENT",
        "temporal_type": "absolute",
        "semantic_type": "mean",
        "creation_time": "2026-06-10 17:50:37",
        "modification_time": "2026-06-10 17:52:46",
        "creator": "saket",
        "start_time": "2024-01-01 00:00:00",
        "end_time": "2027-01-01 00:00:00",
        "granularity": "1 year",
        "north": 248159.844411,
        "south": 203559.011362,
        "east": 671714.551108,
        "west": 619215.133883,
        "bottom": 0.0,
        "top": 0.0,
        "proj": "XY",
        "vector_register": "vector_map_register_e0d7ff31f8dc41de82128f9a9c1e237b",
        "number_of_maps": 3,
        "title": "Wake County Schools",
        "description": "Test dataset for GSoC",
        "command": "...",
        "points": 501,
        "lines": 0,
        "boundaries": 0,
        "centroids": 0,
        "faces": 0,
        "kernels": 0,
        "primitives": 501,
        "nodes": 0,
        "areas": 0,
        "islands": 0,
        "holes": 0,
        "volumes": 0,
        "type": "stvds"
    }
]

@petrasovaa

Copy link
Copy Markdown
Contributor

Some issues found (they might be pre-existing):

  • the line format with columns=all should fail, but it doesn't
  • columns=semantic_label results in error

@petrasovaa

Copy link
Copy Markdown
Contributor

Regarding the different keys for the different types, that is indeed a problem I didn't realize. Currently the CSV with mixed types won't work at all, the header won't match. So unfortunately this is getting more complicated with the "columns=all" option. I think we need to select only keys that intersect for the different groups.

@petrasovaa

Copy link
Copy Markdown
Contributor

I was going back to the "type" option we had previously as one of the options in the columns option. I am now thinking this might be a better option. So if user doesn't request it (e.g. columns=id) it won't show up (which might be more expected).

Now when user requests multiple types and columns is not explicitly specified, the type would show up in JSON (default is columns=all behavior) and in CSV (the default is columns=id,type).

When single type is requested, I am now leaning to omit it (unless explicitly requested with columns option).

Sorry for keep changing the design, this has been somewhat more complicated than I hoped...

Comment thread temporal/t.list/t.list.py Outdated
@petrasovaa

Copy link
Copy Markdown
Contributor

I also realized the output of the function changed, basically breaking the API. Could we return the previous output if single type was selected? So if type is a str, return exactly the old shape (raw row objects, no injected key), if it's a list, return the new dict form.

@saket0187
saket0187 requested review from ninsbl and petrasovaa July 30, 2026 10:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs libraries markdown Related to markdown, markdown files module Python Related code is in Python temporal Related to temporal data processing tests Related to Test Suite

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants