t.upgrade: added from addons #1438

neteler · 2021-03-09T18:04:20Z

copied from https://github.com/OSGeo/grass-addons/tree/master/grass7/temporal/t.upgrade (see addons manual: full URL for core commands grass-addons#265)
added to test_thorough workflow

Aims at addressing #1342

- copied from https://github.com/OSGeo/grass-addons/tree/master/grass7/temporal/t.upgrade - added to test_thorough workflow Aims at addressing #1342

neteler · 2021-03-09T18:42:25Z

The Centos7 workflow fails with:

Traceback (most recent call last):
  File "/github/home/anaconda3/lib/python3.7/ctypes/__init__.py", line 97, in CFUNCTYPE
    return _c_functype_cache[(restype, argtypes, flags)]
KeyError: (<class 'ctypes.c_int'>, (<class 'grass.lib.ctypes_preamble.String'>,), 1)

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/__w/grass/grass/dist.x86_64-pc-linux-gnu/scripts/t.upgrade", line 33, in <module>
    import grass.temporal as tgis
  File "/__w/grass/grass/dist.x86_64-pc-linux-gnu/etc/python/grass/temporal/__init__.py", line 3, in <module>
    from .core import *
  File "/__w/grass/grass/dist.x86_64-pc-linux-gnu/etc/python/grass/temporal/core.py", line 36, in <module>
    from .c_libraries_interface import *
  File "/__w/grass/grass/dist.x86_64-pc-linux-gnu/etc/python/grass/temporal/c_libraries_interface.py", line 20, in <module>
    import grass.lib.gis as libgis
  File "/__w/grass/grass/dist.x86_64-pc-linux-gnu/etc/python/grass/lib/gis.py", line 557, in <module>
    ('checker', CFUNCTYPE(UNCHECKED(c_int), String)),
  File "/github/home/anaconda3/lib/python3.7/ctypes/__init__.py", line 99, in CFUNCTYPE
    class CFunctionType(_CFuncPtr):
TypeError: item 1 in _argtypes_ passes a union by value, which is unsupported.

Any ideas why? (perhaps @nilason or @wenzeslaus know?)

neteler · 2021-03-09T18:51:30Z

OSGeo4W also complains, not sure what's wrong:

Traceback (most recent call last):
  File "D:/a/grass/grass/dist.x86_64-w64-mingw32/scripts/t.upgrade.py", line 33, in <module>
    import grass.temporal as tgis
  File "D:/a/grass/grass/dist.x86_64-w64-mingw32/etc/python/grass/temporal/__init__.py", line 3, in <module>
    from .core import *
  File "D:/a/grass/grass/dist.x86_64-w64-mingw32/etc/python/grass/temporal/core.py", line 36, in <module>
    from .c_libraries_interface import *
  File "D:/a/grass/grass/dist.x86_64-w64-mingw32/etc/python/grass/temporal/c_libraries_interface.py", line 20, in <module>
    import grass.lib.gis as libgis
  File "D:/a/grass/grass/dist.x86_64-w64-mingw32/etc/python/grass/lib/gis.py", line 23, in <module>
    _libs["grass_gis.7.9"] = load_library("grass_gis.7.9")
  File "D:/a/grass/grass/dist.x86_64-w64-mingw32/etc/python/grass/lib/ctypes_loader.py", line 62, in load_library
    return self.load(path)
  File "D:/a/grass/grass/dist.x86_64-w64-mingw32/etc/python/grass/lib/ctypes_loader.py", line 242, in load
    return _WindowsLibrary(path)
  File "D:/a/grass/grass/dist.x86_64-w64-mingw32/etc/python/grass/lib/ctypes_loader.py", line 225, in __init__
    self.cdll = ctypes.cdll.LoadLibrary(path)
  File "C:/msys64/mingw64/lib/python3.8/ctypes/__init__.py", line 451, in LoadLibrary
    return self._dlltype(name)
  File "C:/msys64/mingw64/lib/python3.8/ctypes/__init__.py", line 373, in __init__
    self._handle = _dlopen(self._name, mode)
FileNotFoundError: Could not find module 'D:/a/grass/grass/dist.x86_64-w64-mingw32/lib/libgrass_gis.7.9.dll' (or one of its dependencies). Try using the full path with constructor syntax.

nilason

The Centos7 workflow fails with:

Looks like you found the solution :).

temporal/t.upgrade/t.upgrade.py

petrasovaa · 2021-03-10T03:27:31Z

TypeError: item 1 in argtypes passes a union by value, which is unsupported.

See #305.

wenzeslaus · 2021-03-10T03:54:18Z

Aims at addressing #1342

In which way this would be addressing #1342? As #1342 (comment) says the error happens in a fresh mapset without any prior temporal database. It happens even for locations without any temporal database in any of the mapsets. Even in this PR:

Traceback (most recent call last):
  File "python/grass/temporal/testsuite/unittests_temporal_raster3d_algebra.py", line 40, in setUpClass
    overwrite=True,
  File "etc/python/grass/temporal/open_stds.py", line 213, in open_new_stds
    sp.insert(dbif)
  File "etc/python/grass/temporal/abstract_space_time_dataset.py", line 431, in insert
    dbif.execute_transaction(statement)
  File "etc/python/grass/temporal/core.py", line 1223, in execute_transaction
    return self.connections[mapset].execute_transaction(statement)
  File "etc/python/grass/temporal/core.py", line 1556, in execute_transaction
    self.cursor.executescript(statement)
sqlite3.OperationalError: table str3ds_metadata has no column named number_of_bands
...
sqlite3.OperationalError: no such column: number_of_bands

wenzeslaus

The t.upgrade call in CI seems unnecessary. Otherwise, the original question still remains: Is the version update necessary, i.e., why can't the library support databases with and without band reference?

.github/workflows/test_thorough.sh

veroandreo · 2021-03-11T11:36:04Z

The t.upgrade call in CI seems unnecessary. Otherwise, the original question still remains: Is the version update necessary, i.e., why can't the library support databases with and without band reference?

maybe something sqlite related... AFAIU, the difference among versions is only in a couple of columns in the table... boh

landam · 2021-03-11T14:56:49Z

maybe something sqlite related... AFAIU, the difference among versions is only in a couple of columns in the table... boh

@veroandreo @wenzeslaus To my understanding TGIS DB version property has been designed by @huhabla to reflect changes in the database structure. Even the change seems to be small, the rule seems to be clear (every change in the database structure should be reflected by version change).

landam · 2021-03-11T16:56:19Z

Aims at addressing #1342

In which way this would be addressing #1342? As [#1342 (comment)](https://github.com/OSGeo/grass/issues

See @wenzeslaus newly created #1447

neteler · 2021-03-11T19:14:23Z

The t.upgrade call in CI seems unnecessary.

Ok, reverted in cce2dbf

@wenzeslaus pls remove/fix the unconnected tgis DB in PERMANENT as per #1438 (comment)

wenzeslaus

I have could of comments for the code and two questions:

Why the library doesn't support databases with and without band reference, i.e., versions 2 and 3?
What is now the error message when the database has the wrong version?

temporal/t.upgrade/t.upgrade.html

temporal/t.upgrade/t.upgrade.py

wenzeslaus · 2021-03-12T01:57:55Z

maybe something sqlite related... AFAIU, the difference among versions is only in a couple of columns in the table... boh

@veroandreo @wenzeslaus To my understanding TGIS DB version property has been designed by @huhabla to reflect changes in the database structure. Even the change seems to be small, the rule seems to be clear (every change in the database structure should be reflected by version change).

I think both Vero and I understand that a change in the structure warrants increase in the version number. The question now and in the previous discussion was and is: Is it necessary for the library to not work with older databases and require update or is it possible to work with the older databases and complain only when band reference feature is requested by the user?

wenzeslaus · 2021-03-12T02:13:18Z

The t.upgrade call in CI seems unnecessary.

Ok, reverted in cce2dbf

+1

 _______________________________________
/ My alternative suggestion was to call \
\ it four times!                        /
 ---------------------------------------
        \   ^__^
         \  (oo)\_______
            (__)\       )\/\
                ||----w |
_\|/__\|/__\|/_ ||     ||  _\|/__\|/_

@wenzeslaus pls remove/fix the unconnected tgis DB in PERMANENT as per #1438 (comment)

Oh, poor sample dataset. 😢 What did it do? 😭 It came out blameless from this whole odyssey. With all seriousness, this is far from the only issue with this dataset, but it has alpha2 in the name and it is used only internally for testing. Do we need to finalize it and publish as replacement of the 08 version (which, among other things, involves modifying several tests)? Yes. Is it worth for me to clean some directory there which is not causing any issues? No.

landam · 2021-03-12T22:16:22Z

I think both Vero and I understand that a change in the structure warrants increase in the version number. The question now and in the previous discussion was and is: Is it necessary for the library to not work with older databases and require update or is it possible to work with the older databases and complain only when band reference feature is requested by the user?

#1454

temporal/t.upgrade/t.upgrade.py

wenzeslaus

+1 but check the keywords.

Co-authored-by: Vaclav Petras <wenzeslaus@gmail.com>

* t.upgrade: added from addons - copied from https://github.com/OSGeo/grass-addons/tree/master/grass7/temporal/t.upgrade * Windows: fix t.upgrade call * change grass.temporal to lazy import * remove UTF-8 coding line * black: blank line after import * revert CI related changes * module description fixed * Year pedantry * import grass.script as gs * no need for sys.exit() * global variables options and flags are not used Co-authored-by: Vaclav Petras <wenzeslaus@gmail.com> Co-authored-by: Martin Landa <landa.martin@gmail.com>

neteler added 2 commits March 9, 2021 19:00

t.upgrade: added from addons

484d251

- copied from https://github.com/OSGeo/grass-addons/tree/master/grass7/temporal/t.upgrade - added to test_thorough workflow Aims at addressing #1342

Windows: fix t.upgrade call

e44f48d

change grass.temporal to lazy import

883ba24

nilason reviewed Mar 9, 2021

View reviewed changes

temporal/t.upgrade/t.upgrade.py Outdated Show resolved Hide resolved

temporal/t.upgrade/t.upgrade.py Show resolved Hide resolved

neteler commented Mar 9, 2021

View reviewed changes

temporal/t.upgrade/t.upgrade.py Outdated Show resolved Hide resolved

neteler and others added 2 commits March 9, 2021 21:09

remove UTF-8 coding line

4c6caf5

black: blank line after import

563d6b3

neteler mentioned this pull request Mar 10, 2021

TGRASS: remove bogus band references error msg #1435

Merged

wenzeslaus requested changes Mar 10, 2021

View reviewed changes

.github/workflows/test_thorough.sh Outdated Show resolved Hide resolved

revert CI related changes

cce2dbf

neteler requested a review from wenzeslaus March 11, 2021 19:14

wenzeslaus requested changes Mar 12, 2021

View reviewed changes

temporal/t.upgrade/t.upgrade.html Outdated Show resolved Hide resolved

temporal/t.upgrade/t.upgrade.py Outdated Show resolved Hide resolved

temporal/t.upgrade/t.upgrade.py Outdated Show resolved Hide resolved

temporal/t.upgrade/t.upgrade.py Outdated Show resolved Hide resolved

wenzeslaus mentioned this pull request Mar 12, 2021

[Bug] Many tests fail with issues related to band references #1342

Closed

landam added enhancement New feature or request temporal Related to temporal data processing labels Mar 12, 2021

landam added this to the 8.0.0 milestone Mar 12, 2021

landam added 3 commits March 12, 2021 23:24

module description fixed

e26015b

Year pedantry

254482e

import grass.script as gs

d8023cc

landam added 4 commits March 12, 2021 23:39

no need for sys.exit()

2efec64

global variables options and flags are not used

8775726

manual page: clarified description

23a4d7e

manual: mention G7 and G8

23601ce

landam requested review from wenzeslaus and landam March 12, 2021 23:02

landam approved these changes Mar 12, 2021

View reviewed changes

landam requested a review from veroandreo March 12, 2021 23:04

black fun

89600c5

wenzeslaus reviewed Mar 13, 2021

View reviewed changes

temporal/t.upgrade/t.upgrade.py Outdated Show resolved Hide resolved

wenzeslaus approved these changes Mar 13, 2021

View reviewed changes

remove keyword

8bca27f

Co-authored-by: Vaclav Petras <wenzeslaus@gmail.com>

veroandreo mentioned this pull request Mar 15, 2021

TGIS: Make database v3 backward compatible with v2 #1454

Merged

veroandreo approved these changes Mar 15, 2021

View reviewed changes

neteler merged commit 98347de into OSGeo:master Mar 15, 2021

neteler deleted the t_upgrade_module branch March 15, 2021 16:15

landam added a commit to landam/grass-addons that referenced this pull request Mar 19, 2021

t.upgrade now available in GRASS core, OSGeo/grass#1438

787ce63

landam mentioned this pull request Mar 19, 2021

t.upgrade now available in GRASS core OSGeo/grass-addons#487

Merged

landam added a commit to OSGeo/grass-addons that referenced this pull request Mar 21, 2021

t.upgrade now available in GRASS core, OSGeo/grass#1438 (#487)

7ba69d8

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

t.upgrade: added from addons #1438

t.upgrade: added from addons #1438

neteler commented Mar 9, 2021 •

edited

Loading

neteler commented Mar 9, 2021

neteler commented Mar 9, 2021

nilason left a comment

petrasovaa commented Mar 10, 2021

wenzeslaus commented Mar 10, 2021

wenzeslaus left a comment •

edited

Loading

veroandreo commented Mar 11, 2021

landam commented Mar 11, 2021

landam commented Mar 11, 2021

neteler commented Mar 11, 2021

wenzeslaus left a comment

wenzeslaus commented Mar 12, 2021

wenzeslaus commented Mar 12, 2021

landam commented Mar 12, 2021

wenzeslaus left a comment

t.upgrade: added from addons #1438

t.upgrade: added from addons #1438

Conversation

neteler commented Mar 9, 2021 • edited Loading

neteler commented Mar 9, 2021

neteler commented Mar 9, 2021

nilason left a comment

Choose a reason for hiding this comment

petrasovaa commented Mar 10, 2021

wenzeslaus commented Mar 10, 2021

wenzeslaus left a comment • edited Loading

Choose a reason for hiding this comment

veroandreo commented Mar 11, 2021

landam commented Mar 11, 2021

landam commented Mar 11, 2021

neteler commented Mar 11, 2021

wenzeslaus left a comment

Choose a reason for hiding this comment

wenzeslaus commented Mar 12, 2021

wenzeslaus commented Mar 12, 2021

landam commented Mar 12, 2021

wenzeslaus left a comment

Choose a reason for hiding this comment

neteler commented Mar 9, 2021 •

edited

Loading

wenzeslaus left a comment •

edited

Loading