-
-
Notifications
You must be signed in to change notification settings - Fork 332
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: new module t.copy #1930
TGIS: new module t.copy #1930
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice addition!
@veroandreo Thanks for the instant review! |
temporal/t.copy/t.copy.py
Outdated
% (maptype, row["name"], mapset) | ||
) | ||
|
||
kwargs = {maptype: "%s,%s" % (row["id"], row["name"])} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not familiar with this syntax, what is the maptype
parameter in g.copy
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The syntax of g.copy
is g.copy [raster=from,to] [raster_3d=from,to] [vector=from,to] ...
. Here I use kwargs to avoid an if ... elif ...
construct. maptype
is here one of raster, raster_3d, vector.
"""Remove the temporary region""" | ||
cls.del_temp_region() | ||
|
||
def setUp(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isn't all of this already done in the setUpClass method?
If the <em>-c</em> flag is given, the maps of the old space time dataset | ||
will also be copied to the current mapset, otherwise the original maps | ||
will be simply registered in the temporal database. The new copies will | ||
have the same name as the old maps. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if there is already a raster map with the same name? Does an error occur then, and can I overwrite that with --o
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this case a fatal error is issued. Maybe overwriting the existing map in case of --o
could be allowed?
# %option G_OPT_STDS_TYPE | ||
# % guidependency: input | ||
# % guisection: Required | ||
# % options: strds, str3ds, stvds |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In other modules like t.create
is the default strds
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The default is here also strds
, see https://github.com/OSGeo/grass/blob/main/lib/gis/parser_standard_options.c#L884
|
||
inname = input | ||
inmapset = None | ||
if "@" in input: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't there always have to be an @
in the input, because you copy a STRDS from another mapset into the current one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, because as for g.copy
, you might want to create a copy of a dataset that is already in the current mapset. t.copy
tries to follow the behaviour of g.copy
.
I've downloaded the
It only gets copied with full name, not as indicated in the manual page. Could the error be removed? It's misleading. In any case, the copied strds is empty:
Is this the desired behaviour? What's the point of making a copy if it will be empty? Should it read the maps from modis_lst? Then, when using
|
As I wrote in the description, this PR requires PR #1924 to work properly across mapsets.
and
should then dissapear. |
ops... I clearly overlooked that, apologies for the noise then... I'll test properly asap |
I tried once more by downloading the patches and applying them sequentially (first 1924 then this one) and recompiling.
Summary according to testing:
Is this the expected behavior? |
OK, unfortunately TGIS does not use the mechanism of the GRASS C library to find datae elements.
TGIS is mapset specific, that is, the existing raster maps in a different mapset must be registered in the TGIS db of the current mapset. Apparently, #1924 does not work as expected.
Exactly, the original motivation was to avoid data duplication, to be precise, avoid the need to copy the actual raster maps to the current mapset in order to create a new STDS. I will have time to take care of these issues by the end of the coming week. |
Changing milestone to 8.2.0. Independent and thus can be backported if really needed in 8.0, but as a feature/addtion, it does not need to be in 8.0.0. |
@veroandreo I have updated the manual and fixed open issues with The issues mentioned by you:
should be resolved by now. I think that |
Co-authored-by: Veronica Andreo <veroandreo@gmail.com>
Co-authored-by: Veronica Andreo <veroandreo@gmail.com>
Co-authored-by: Veronica Andreo <veroandreo@gmail.com>
Co-authored-by: Veronica Andreo <veroandreo@gmail.com>
Co-authored-by: Guido Riembauer <62383722+griembauer@users.noreply.github.com>
Hi @metzm! Great news! I'll test this weekend :) |
All works as expected! Cool!
|
* new module t.copy Co-authored-by: Veronica Andreo <veroandreo@gmail.com> Co-authored-by: Guido Riembauer <62383722+griembauer@users.noreply.github.com>
* new module t.copy Co-authored-by: Veronica Andreo <veroandreo@gmail.com> Co-authored-by: Guido Riembauer <62383722+griembauer@users.noreply.github.com>
* new module t.copy Co-authored-by: Veronica Andreo <veroandreo@gmail.com> Co-authored-by: Guido Riembauer <62383722+griembauer@users.noreply.github.com>
This new module
t.copy
creates a copy of a space-time dataset. It can also copy datasets from a different mapset to the current mapset. Optionally, maps registered in the dataset are also copied. By default, only a new space-time dataset is created.This PR requires PR #1924 to work properly across mapsets.