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

TGIS: Make database v3 backward compatible with v2 #1454

Merged
merged 11 commits into from
Dec 4, 2021

Conversation

landam
Copy link
Member

@landam landam commented Mar 12, 2021

Currently TGIS DB created in GRASS <7.9 (version 2) is not supported by GRASS 7.9 (version 3).

> t.info v2
ERROR: Unsupported temporal database: version mismatch.
The format of your actual temporal database is not supported any more.
Please create a backup of your temporal database to avoid lossing data.
SOLUTION: Run t.upgrade command installed from GRASS Addons in order to
upgrade your temporal database.
Supported temporal database version is: 2
Current temporal database info:
DBMI interface:..... sqlite3
Temporal database:.. /home/martin/grassdata/tgis/PERMANENT/tgis/sqlite.db

This PR introduces first step for backward compatibility:

t.info v2
WARNING: Temporal database version mismatch detected.
         Please create a backup of your temporal database to avoid lossing
         data.
         SOLUTION: Run t.upgrade command installed from GRASS Addons in
         order to upgrade your temporal database.
         Supported temporal database version is: 3
         Current temporal database info:
         DBMI interface:..... sqlite3
         Temporal database:..
         /home/martin/grassdata/tgis/PERMANENT/tgis/sqlite.db
 +-------------------- Space Time Raster Dataset -----------------------------+
 |                                                                            |
...
 | Number of registered maps:.. 1
 |
 | Title:
 | x
 | Description:
 | y
 | Command history:
 | # 2021-03-12 22:19:52 
 | t.create v2 tit="x" d="y"
 | # 2021-03-12 22:23:53 
 | t.register input="v2" maps="rast" start="2020-01-01"
 |     --o
 | 
 +----------------------------------------------------------------------------+

Note that it's a draft which need to be tested.

@landam landam marked this pull request as draft March 12, 2021 22:03
@landam landam self-assigned this Mar 12, 2021
@landam landam added enhancement New feature or request temporal Related to temporal data processing labels Mar 12, 2021
@landam landam added this to the 8.0.0 milestone Mar 12, 2021
@landam landam changed the title TGIS v3 backward compatible with v2 TGIS DB v3 backward compatible with v2 Mar 12, 2021
python/grass/temporal/core.py Outdated Show resolved Hide resolved
python/grass/temporal/core.py Outdated Show resolved Hide resolved
python/grass/temporal/core.py Outdated Show resolved Hide resolved
python/grass/temporal/metadata.py Outdated Show resolved Hide resolved
python/grass/temporal/core.py Outdated Show resolved Hide resolved
Co-authored-by: Markus Neteler <neteler@gmail.com>
@wenzeslaus wenzeslaus added the blocker Blocking a release label Oct 5, 2021
@wenzeslaus wenzeslaus removed the blocker Blocking a release label Nov 12, 2021
@landam landam marked this pull request as ready for review November 26, 2021 17:47
@landam landam marked this pull request as draft November 26, 2021 18:00
@landam landam marked this pull request as ready for review November 26, 2021 18:26
@landam
Copy link
Member Author

landam commented Nov 26, 2021

Current message:

WARNING: Temporal database version mismatch detected.
         Please create a backup of your temporal database to avoid losing
         data.
         Run t.upgrade command to upgrade your temporal database.
         Supported temporal database version is: 3
         Your existing temporal database version: 2
         Current temporal database info:
         DBMI interface:..... sqlite3
         Temporal database:..
         /home/martin/grassdata/tgis/PERMANENT/tgis/sqlite.db

@landam landam added the blocker Blocking a release label Nov 27, 2021
@landam
Copy link
Member Author

landam commented Nov 27, 2021

Attempt to modify strds still fails

t.register input=test maps=x1 start=2020-01-01
...
(SELECT id FROM raster_map_register_147b8e68ee454031a9d993dc5cf31194)
) WHERE id = 'test@PERMANENT';
UPDATE st
ERROR: no such column: semantic_label

due to https://github.com/landam/grass/blob/tgisdb3_backward_compatibility/lib/temporal/SQL/update_strds_metadata_template.sql#L45

@marisn Do we want to support modification of strds created by GRASS v7 in v8?

@landam
Copy link
Member Author

landam commented Nov 27, 2021

@marisn We could either split files:

  • update_strds_metadata_template.sql (common)
  • update_strds_metadata_template_v3.sql (version 3+)

or to use inline modification as applied in 481f681 (but more robust). What do you think?

@landam landam marked this pull request as draft November 27, 2021 16:19
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.

Thanks for addressing what I raised earlier, esp. the important message-related things. I like how it looks like now, but I can review again if needed.

python/grass/temporal/abstract_space_time_dataset.py Outdated Show resolved Hide resolved
Copy link
Contributor

@veroandreo veroandreo left a comment

Choose a reason for hiding this comment

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

I like the wording in the new message.

I'm still somehow confused though. AFAIU, if there's a version mismatch (supported=3, current=2) the user needs to run t.upgrade, is that what we call backward compatibility? I'd have assumed compatibility meant some background update of versions transparent to the user.

Also, instead of number_of_bands shouldn't we use something with semantic_label for consistency?

@landam
Copy link
Member Author

landam commented Nov 30, 2021

I'm still somehow confused though. AFAIU, if there's a version mismatch (supported=3, current=2) the user needs to run t.upgrade, is that what we call backward compatibility? I'd have assumed compatibility meant some background update of versions transparent to the user.

By backward compatibility we mean that there is no need to run t.upgrade if you are not interested about v3 new features (like semantic_labels).

Also, instead of number_of_bands shouldn't we use something with semantic_label for consistency?

Probably yes, but it's for separated PR. @marisn Any opinion?

@marisn
Copy link
Contributor

marisn commented Dec 1, 2021

Also, instead of number_of_bands shouldn't we use something with semantic_label for consistency?

Probably yes, but it's for separated PR. @marisn Any opinion?

I'm not that familiar with TGIS internals to comment if it makes sense or not. We must keep in mind is that imagery group members are called bands. i.bands.library has band references as a subclass/a kind of semantic labels and thus having bands, band references is just fine when dealing with remote sensing data.

@veroandreo
Copy link
Contributor

Also, instead of number_of_bands shouldn't we use something with semantic_label for consistency?

Probably yes, but it's for separated PR. @marisn Any opinion?

I'm not that familiar with TGIS internals to comment if it makes sense or not. We must keep in mind is that imagery group members are called bands. i.bands.library has band references as a subclass/a kind of semantic labels and thus having bands, band references is just fine when dealing with remote sensing data.

Agreed, but time series of raster data (STRDS) can consist of any type of raster maps, be them satellite imagery or not... this was the whole point of changing band reference to something more general

@landam
Copy link
Member Author

landam commented Dec 2, 2021

Quick test: create strds and register 1st Sentinel band

  1. In GRASS 8 t.info reports:
 | Number of registered bands:. 1
 | Band names:................. S2_1
 | Number of registered maps:.. 1
  1. Attempt to run t.info on the same strds in GRASS 7.8.7dev fails with:
ERROR: Unsupported temporal database: version mismatch.
The format of your actual temporal database is not supported any more.
Solution: You need to export it by restoring the GRASS GIS version used for
creating this DB. From there, create a backup of your temporal database to
avoid the loss of your temporal data.
Notes: Use t.rast.export and t.vect.export to make a backup of your
existing space time datasets.To safe the timestamps of your existing maps
and space time datasets, use t.rast.list, t.vect.list and t.rast3d.list.
You can register the existing time stamped maps easily if you export
columns=id,start_time,end_time into text files and use t.register to
register them again in new created space time datasets (t.create). After
the backup remove the existing temporal database, a new one will be created
automatically.
Supported temporal database version is: 2
Current temporal database info:
DBMI interface:..... sqlite3
Temporal database:..
/home/martin/grassdata/tgis_sentinel/tgis3/tgis/sqlite.db

@metzm Should be this message modified (in separated PR) to promote t.downgrade in GRASS 7.8?

  1. Create new strds in GRASS 7.8
| Number of registered maps:.. 1
  1. Open strds created by GRASS 7.8 in GRASS 8:
WARNING: Temporal database version mismatch detected.
         Please create a backup of your temporal database to avoid losing
         data.
         Run t.upgrade command to upgrade your temporal database.
         Supported temporal database version is: 3
         Your existing temporal database version: 2
         Current temporal database info:
         DBMI interface:..... sqlite3
         Temporal database:..
         /home/martin/grassdata/tgis_sentinel/tgis2/tgis/sqlite.db
...
 | Number of registered bands:. None
 | Band names:................. None
 | Number of registered maps:.. 1

Ready for review and merge if approved.

@landam landam marked this pull request as ready for review December 2, 2021 15:59
@landam
Copy link
Member Author

landam commented Dec 2, 2021

Agreed, but time series of raster data (STRDS) can consist of any type of raster maps, be them satellite imagery or not... this was the whole point of changing band reference to something more general

@marisn @veroandreo So what about changing

 | Number of registered bands:. 1
 | Band names:................. S2_1

to

 | Number of reg. semantic labels:. 1
 | Semantic labels:................. S2_1

I can prepare separated PR if you agree.

@marisn
Copy link
Contributor

marisn commented Dec 2, 2021

Agreed, but time series of raster data (STRDS) can consist of any type of raster maps, be them satellite imagery or not... this was the whole point of changing band reference to something more general

@marisn @veroandreo So what about changing

 | Number of registered bands:. 1
 | Band names:................. S2_1

to

 | Number of reg. semantic labels:. 1
 | Semantic labels:................. S2_1

I can prepare separated PR if you agree.

Fine for me.

@landam
Copy link
Member Author

landam commented Dec 2, 2021

If no objection I will merge this PR in the next days.

@veroandreo
Copy link
Contributor

veroandreo commented Dec 3, 2021

Some suggestions:

Quick test: create strds and register 1st Sentinel band

  1. In GRASS 8 t.info reports:
 | Number of registered bands:. 1
 | Band names:................. S2_1
 | Number of registered maps:.. 1
  1. Attempt to run t.info on the same strds in GRASS 7.8.7dev fails with:
ERROR: Unsupported temporal database: version mismatch.
The format of your actual temporal database is not supported any more.

Since this would be a downgrade, I'd remove the "any more" part which indicates that you have something old not supported. Instead, in this case, users will have a new format and try to read it in an "old" grass.

Solution: You need to export it by restoring the GRASS GIS version used for
creating this DB. From there, create a backup of your temporal database to
avoid the loss of your temporal data.
Notes: Use t.rast.export and t.vect.export to make a backup of your
existing space time datasets.To safe the timestamps of your existing maps

Replace "safe" by save

and space time datasets, use t.rast.list, t.vect.list and t.rast3d.list.
You can register the existing time stamped maps easily if you export
columns=id,start_time,end_time into text files and use t.register to
register them again in new created space time datasets (t.create). After
the backup remove the existing temporal database, a new one will be created
automatically.
Supported temporal database version is: 2
Current temporal database info:
DBMI interface:..... sqlite3
Temporal database:..
/home/martin/grassdata/tgis_sentinel/tgis3/tgis/sqlite.db


@metzm Should be this message modified (in separated PR) to promote `t.downgrade` in GRASS 7.8?

Sorry to step in here, but, yes, I'd promote t.downgrade as the main and most straightforward solution. Indeed, I'd shrink the message above as the case for upgrade.

@landam landam merged commit facbacc into OSGeo:main Dec 4, 2021
@landam landam deleted the tgisdb3_backward_compatibility branch December 4, 2021 15:14
landam added a commit that referenced this pull request Dec 4, 2021
Co-authored-by: Markus Neteler <neteler@gmail.com>
@landam
Copy link
Member Author

landam commented Dec 4, 2021

Sorry to step in here, but, yes, I'd promote t.downgrade as the main and most straightforward solution. Indeed, I'd shrink the message above as the case for upgrade.

See #2002

@wenzeslaus wenzeslaus changed the title TGIS DB v3 backward compatible with v2 TGIS: Make database v3 backward compatible with v2 Apr 27, 2022
ninsbl pushed a commit to ninsbl/grass that referenced this pull request Oct 26, 2022
Co-authored-by: Markus Neteler <neteler@gmail.com>
ninsbl pushed a commit to ninsbl/grass that referenced this pull request Feb 17, 2023
Co-authored-by: Markus Neteler <neteler@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocker Blocking a release enhancement New feature or request temporal Related to temporal data processing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants