Skip to content
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

temporal dbif for current mapset only #2448

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

metzm
Copy link
Contributor

@metzm metzm commented Jun 17, 2022

Sometimes only the current mapset must be used by TGIS, particularly if the TGIS db is going to be modified. TGIS dbs in other mapsets can/should not be modified, only queried.

This PR adds functionality to initialize a SQLDatabaseInterfaceConnection with only the current mapset.

@metzm metzm added enhancement New feature or request temporal Related to temporal data processing Python Related code is in Python labels Jun 17, 2022
@metzm metzm added this to the 8.4.0 milestone Jun 17, 2022
@metzm metzm requested a review from wenzeslaus June 17, 2022 17:05
Copy link
Member

@wenzeslaus wenzeslaus left a comment

Choose a reason for hiding this comment

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

This looks reasonable. I have some documentation and stylistic notes.

python/grass/temporal/core.py Outdated Show resolved Hide resolved
python/grass/temporal/core.py Show resolved Hide resolved
@metzm metzm force-pushed the temporal_only_current_mapset branch from f1e11da to 53baae9 Compare June 20, 2022 08:34
wenzeslaus
wenzeslaus previously approved these changes Jun 20, 2022
Copy link
Member

@wenzeslaus wenzeslaus left a comment

Choose a reason for hiding this comment

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

As far as the code goes, this is good to go.

@metzm
Copy link
Contributor Author

metzm commented Jun 20, 2022

A related test for TGIS mapset access is failing. This needs to be fixed first before merging.

@metzm
Copy link
Contributor Author

metzm commented Jun 23, 2022

A related test for TGIS mapset access is failing. This needs to be fixed first before merging.

This will require a bit more effort because GRASS TGIS still expects at various different places that a raster/raster3d/vector map can only be registered in a TGIS db in the same mapset where this map is located. This prevents registering maps from a different mapset in a space-time dataset in the current mapset and subsequent modifications of this space-time dataset in the current mapset.

A solution would be to not only store information about the mapset of a map, but also information about the mapset of the space-time dataset that is currently used and where this map is registered in the internal structures of the temporal framework.

The historical background is that a map could only be registered in a space-time dataset if both are in the current mapset.

@wenzeslaus
Copy link
Member

At this point, this seems like something for 8.4.0.

@neteler
Copy link
Member

neteler commented Apr 16, 2023

@metzm shall I bump this PR to 8.4.0?

@wenzeslaus wenzeslaus modified the milestones: 8.3.0, 8.4.0 May 12, 2023
@metzm metzm force-pushed the temporal_only_current_mapset branch from 22e8ba7 to e0692ae Compare September 3, 2023 09:44
@landam
Copy link
Member

landam commented Nov 20, 2023

@metzm Please consider merging this PR.

@wenzeslaus wenzeslaus modified the milestones: 8.4.0, 8.5.0 Apr 26, 2024
@echoix echoix added the needs rebase Rebase to or merge with the latest base branch is needed label Nov 7, 2024
@echoix
Copy link
Member

echoix commented Nov 10, 2024

I tried to solve conflicts, but I couldn't make sense of the changes in regards to changing self.mapset to self.data_mapset, that didn't seem used correctly. So I aborted it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request libraries module needs rebase Rebase to or merge with the latest base branch is needed Python Related code is in Python temporal Related to temporal data processing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants