Skip to content

t.list: Add mapset options for t.list#7631

Merged
ninsbl merged 10 commits into
OSGeo:mainfrom
saket0187:Add-mapset-option-in-t.list
Jul 8, 2026
Merged

t.list: Add mapset options for t.list#7631
ninsbl merged 10 commits into
OSGeo:mainfrom
saket0187:Add-mapset-option-in-t.list

Conversation

@saket0187

@saket0187 saket0187 commented Jul 3, 2026 •
edited by ninsbl
Loading

Copy link
Copy Markdown
Contributor

This PR adds the mapset option mentioned in #7628

Fixes: #7622
Fixes: #7576

@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 3, 2026
ninsbl
ninsbl requested changes Jul 4, 2026
View reviewed changes

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

Thanks, I had not thought that so many changes were needed, but you seemed to have identified required changes correctly.

In general the approach looks good, and it did at least helo me understand some TGIS fundamentals a bit better.

I have do some suggestions though. See the inline comments.

@petrasovaa what do you think.

Comment thread python/grass/temporal/core.py Outdated
Comment thread python/grass/temporal/core.py Outdated
Comment thread python/grass/temporal/core.py Outdated
Comment thread python/grass/temporal/core.py Outdated
Comment thread python/grass/temporal/list_stds.py Outdated
Comment thread temporal/t.list/t.list.md Outdated
This was referenced Jul 4, 2026
Merged
Open
@saket0187

saket0187 commented Jul 5, 2026

Copy link
Copy Markdown
Contributor Author

@ninsbl, is this what you were asking for?

@saket0187 saket0187 requested a review from ninsbl July 5, 2026 17:50
@ninsbl

ninsbl commented Jul 5, 2026

Copy link
Copy Markdown
Member

@ninsbl, is this what you were asking for?

Partly.

In order to fix #7576 we need to add something like this to tgis.init(require_tgisdb_in_current_mapset: bool = True) that can be set to False for t.list, but that is probably a different PR.

See otherwise my comments in the review for the other stuff...

ninsbl
ninsbl requested changes Jul 5, 2026
View reviewed changes
Comment thread python/grass/temporal/core.py Outdated
Comment thread python/grass/temporal/core.py Outdated
Comment thread python/grass/temporal/core.py Outdated
Comment thread python/grass/temporal/list_stds.py Outdated
Comment thread python/grass/temporal/list_stds.py
Comment thread python/grass/temporal/list_stds.py Outdated
Comment thread temporal/t.list/t.list.py Outdated
@ninsbl ninsbl mentioned this pull request Jul 6, 2026
Closed
@saket0187 saket0187 requested a review from ninsbl July 6, 2026 14:38
github-actions[bot]
github-actions Bot reviewed Jul 6, 2026
View reviewed changes
Comment thread temporal/t.list/t.list.py Outdated
Comment thread temporal/t.list/t.list.py Outdated
github-actions[bot]
github-actions Bot reviewed Jul 6, 2026
View reviewed changes
Comment thread temporal/t.list/t.list.py
@ninsbl ninsbl mentioned this pull request Jul 7, 2026
Merged
@ninsbl

ninsbl commented Jul 7, 2026

Copy link
Copy Markdown
Member

Hi again, @saket0187. I pushed some more suggestions to your fork. With #7645 merged, updating the branch here should now make the tests pass (I canceled them as they currently would fail anyway).

It would be great if you could test against #7645 locally and report back. This should also properly fix #7580.

Sorry for interfering with your PR, I just would like to fix the state and behavior of t.list ASAP and had an open window right now...

@ninsbl ninsbl requested a review from petrasovaa July 7, 2026 11:39
ninsbl
ninsbl approved these changes Jul 7, 2026
View reviewed changes

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

Tests are now passing. Seems good to me.
@petrasovaa , @echoix or @wenzeslaus , since I tampered with the PR could one of you have a second look and merge in case?

@ninsbl ninsbl requested a review from echoix July 7, 2026 18:24
Valyrian-Code added a commit to Valyrian-Code/grass that referenced this pull request Jul 8, 2026
@Valyrian-Code
t.list: keep only the regression test, the fix lands in OSGeo#7631
8d824ed
@ninsbl

ninsbl commented Jul 8, 2026

Copy link
Copy Markdown
Member

With also @Valyrian-Code testing the code successfully, I think it is acceptable to merge also without an additional review, also because this PR fixes a newly introduced bug (#7622).

@ninsbl ninsbl merged commit dcb3324 into OSGeo:main Jul 8, 2026
26 checks passed
@github-actions github-actions Bot added this to the 8.6.0 milestone Jul 8, 2026
echoix pushed a commit that referenced this pull request Jul 10, 2026
@Valyrian-Code
t.list: add regression test for listing from a mapset without a tempo…
a53127d 
…ral database (#7640)

* t.list: check for temporal databases in all accessible mapsets

* t.list: keep only the regression test, the fix lands in #7631
@gkneighb gkneighb mentioned this pull request Jul 12, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@github-actions github-actions[bot] github-actions[bot] left review comments
@ninsbl ninsbl ninsbl approved these changes
@petrasovaa petrasovaa Awaiting requested review from petrasovaa
@echoix echoix Awaiting requested review from echoix

Assignees

No one assigned

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

Milestone

8.6.0

2 participants

@saket0187 @ninsbl