Opened 17 years ago
Last modified 4 years ago
#70 new task
imagery modules: strip @mapset part
Reported by: | hamish | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | 8.0.0 |
Component: | Imagery | Version: | svn-trunk |
Keywords: | imagery | Cc: | Paula.Sankelo@… |
CPU: | Unspecified | Platform: | Unspecified |
Description
Hi,
if you call i.target from a GUI and select a group it appends @mapset. If you run it like that it takes the full name literally and instead of updating the group's target it creates a new group of name "group@mapset". Either i.target or I_put_target() should check that if the @mapset part is given it refers to the current mapset, then strip off the @mapset part.
Otherwise you end up with
GRASS> g.list group group group@mapset group@mapset@mapset etc.
I changed the group option gisprompt from old,group,group to any,gr,gr. This makes the GUI group picker button go away, but perhaps the GUI picker button should be coded to remain in that case?
Or to solve this should we try changing the gisprompt to mapset,gr,gr so it calls G_ask_in_mapset()?
Hamish
Attachments (3)
Change History (27)
comment:1 by , 17 years ago
comment:2 by , 17 years ago
follow-up: 4 comment:3 by , 17 years ago
Cc: | added |
---|---|
Keywords: | imagery added |
Summary: | i.target from GUI: strip @mapset part → imagery modules: strip @mapset part |
i.group, i.target modified in SVN to handle @mapset qualified names in r30802. i.class done in r30803.
As this is a systemic problem a better approach would be to improve the imagery libs to deal with that -- renaming bug and leaving it open.
I don't really use the imagery modules so I'm not sure what other modules are badly crippled from this. Proably most, but as mentioned above the way to tackle that is to fix it properly in the imagery lib fns.
e.g. listing maps contained in a group from another mapset should work.
Hamish
follow-up: 5 comment:4 by , 17 years ago
comment:5 by , 17 years ago
Replying to glynn:
I have attached a more extensive version of the previous patch.
attachment:ticket:70:imagery-mapset.patch
This should cover all of the cases where filenames were being constructed manually.
There remains the issue that the imagery library can only handle groups in the current mapset, for both read and write. Most of that could be fixed by replacing G_mapset() with "". However, the I_find_* functions really need _new and _old versions, as (unlike the G_find_* functions) they don't accept a mapset argument.
comment:6 by , 17 years ago
It appears that i.group is now broken:
http://lists.osgeo.org/pipermail/grass-user/2008-May/044688.html
follow-up: 8 comment:7 by , 17 years ago
(i.group functional again).
I tried to use i.class, NC data set:
i.group mylsat7_2000 sub=mylsat7_2000 in=lsat7_2000_10,lsat7_2000_20,lsat7_2000_30,lsat7_2000_40,lsat7_2000_50,lsat7_2000_61,lsat7_2000_70,lsat7_2000_80 g.region rast=lsat7_2000_40 -p d.mon x0 i.class map=lsat7_2000_40 group=mylsat7_2000 subgroup=mylsat7_2000 outsig=lsat7_2000_sig RESULT SIGNATURE WARNING: unable to create signature file lsat7_2000_sig for subgroup mylsat7_2000 of group mylsat7_2000 ERROR: Unable to open output signature file 'lsat7_2000_sig'
It seems to be an issue in lib/imagery/sigfile.c (devel_grass6 branch).
Markus
comment:8 by , 17 years ago
Replying to neteler:
(i.group functional again).
> I tried to use i.class, NC data set: > WARNING: unable to create signature file lsat7_2000_sig for subgroup > mylsat7_2000 of group mylsat7_2000 > ERROR: Unable to open output signature file 'lsat7_2000_sig'
It seems to be an issue in lib/imagery/sigfile.c (devel_grass6 branch).
comment:11 by , 17 years ago
follow-up: 13 comment:12 by , 17 years ago
While i.class now works from cmd line, it still fails interactively:
GRASS 6.4.svn (nc_spm_07):~ > i.class OPTION: Name of raster map to be displayed key: map format: name required: YES Enter the name of an existing raster file Enter 'list' for a list of existing raster files Hit RETURN to cancel request > lsat7_2002_40 <lsat7_2002_40> OPTION: Name of input imagery group key: group format: name required: YES Enter the name of an existing group file Enter 'list' for a list of existing group files Hit RETURN to cancel request > mylsat7_2000 <mylsat7_2000> OPTION: Name of input imagery subgroup key: subgroup format: name required: YES Enter the name of an existing subgroup file Enter 'list' for a list of existing subgroup files Hit RETURN to cancel request > list <list> ---------------------------------------------- no subgroup files available in current mapset ---------------------------------------------- Enter the name of an existing subgroup file Enter 'list' for a list of existing subgroup files Hit RETURN to cancel request > mylsat7_2000 <mylsat7_2000> ** mylsat7_2000 - not found ** Enter the name of an existing subgroup file Enter 'list' for a list of existing subgroup files Hit RETURN to cancel request >
I suspect a bug in grass/trunk/lib/imagery/find.c
Markus
comment:13 by , 17 years ago
Replying to neteler:
While i.class now works from cmd line, it still fails interactively:
That's a separate issue, which was introduced in r24069.
<list>
no subgroup files available in current mapset
If you read that literally, it should give you a clue what's going on here.
I suspect a bug in grass/trunk/lib/imagery/find.c
Nope; it's a limitation of G_parser(), which cannot handle subgroups. Essentially, "old,subgroup,subgroup" isn't a valid ->gisprompt setting.
G_parser() itself knows nothing of the imagery library, and there is no way for individual modules to extend it. When G_parser() performs interactive input for an option with "old,<element>,<description>" as the ->gisprompt value, it just checks for the file in the <element> directory. Similarly, if you type "list", it just lists the <element> directory.
Well, mapsets don't have a "subgroup" directory, as subgroups belong to specific groups, not the mapset. Beyond that, G_parser() would have no way of knowing which group the subgroup belongs to (it cannot know that the value of the group= option indicates this).
Ultimately, you'll have to just remove the ->gisprompt setting, leaving the option as just a string.
comment:14 by , 17 years ago
comment:15 by , 17 years ago
It seems that i.atcorr has some mapset related problems, too:
GRASS 6.4.svn (nc_spm_07):~ > g.gisenv GISDBASE=/home/neteler/grassdata LOCATION_NAME=nc_spm_07 MAPSET=sqlite DEBUG=0 GRASS_GUI=text GRASS 6.4.svn (nc_spm_07):~ > g.mapsets -p sqlite PERMANENT GRASS 6.4.svn (nc_spm_07):~ > i.atcorr -b iimg=lsat5_1987_60@landsat iscl=-6.978740,191.600 icnd=ddddd oimg=test_p112r056_b1.rad.atcorr oscl=0,255 G__open(r): mapset (sqlite) doesn't match xmapset (landsat) G__open(r): mapset (sqlite) doesn't match xmapset (landsat) WARNING: Unable to open header file for raster map <lsat5_1987_60@landsat@sqlite> ERROR: Unable to retreive header dat for input image GRASS 6.4.svn (nc_spm_07):~ > g.mapsets add=landsat GRASS 6.4.svn (nc_spm_07):~ > i.atcorr -b iimg=lsat5_1987_60 iscl=-6.978740,191.600 icnd=ddddd oimg=test_p112r056_b1.rad.atcorr oscl=0,255 WARNING: Unable to open header file for raster map <lsat5_1987_60@sqlite> ERROR: Unable to retreive header dat for input image
Markus
comment:16 by , 17 years ago
follow-ups: 18 20 comment:17 by , 17 years ago
I have found a new related bug: -r of i.group fails.
g.gisenv set=DEBUG=3 GRASS 6.4.svn (nc_spm_07): > i.group -r mylsat7_2000 sub=mylsat7_2000 in=lsat7_2000_61 D3/3: remove_subgroup_files: ref_tmp.nfiles 8 D3/3: tmp_name lsat7_2000_61, ref_tmp.file[0].name: lsat7_2000_10 D3/3: mapset neteler, ref_tmp.file[0].mapset: landsat D3/3: tmp_name lsat7_2000_61, ref_tmp.file[1].name: lsat7_2000_20 D3/3: mapset neteler, ref_tmp.file[1].mapset: landsat D3/3: tmp_name lsat7_2000_61, ref_tmp.file[2].name: lsat7_2000_30 D3/3: mapset neteler, ref_tmp.file[2].mapset: landsat D3/3: tmp_name lsat7_2000_61, ref_tmp.file[3].name: lsat7_2000_40 D3/3: mapset neteler, ref_tmp.file[3].mapset: landsat D3/3: tmp_name lsat7_2000_61, ref_tmp.file[4].name: lsat7_2000_50 D3/3: mapset neteler, ref_tmp.file[4].mapset: landsat D3/3: tmp_name lsat7_2000_61, ref_tmp.file[5].name: lsat7_2000_61 D3/3: mapset neteler, ref_tmp.file[5].mapset: landsat D3/3: tmp_name lsat7_2000_61, ref_tmp.file[6].name: lsat7_2000_70 D3/3: mapset neteler, ref_tmp.file[6].mapset: landsat D3/3: tmp_name lsat7_2000_61, ref_tmp.file[7].name: lsat7_2000_80 D3/3: mapset neteler, ref_tmp.file[7].mapset: landsat D1/3: writing subgroup REF D2/3: G__home home = /home/neteler WARNING: No raster map removed i.group complete.
It compares the wrong mapsets here (G_mapset() should possibly not used on the left side, see function remove_subgroup_files() of main.c of i.group.
Markus
follow-up: 19 comment:18 by , 17 years ago
Replying to neteler:
I have found a new related bug: -r of i.group fails.
G_mapset() returns a pointer to a static buffer. If you modify this buffer, you modify the return value for all subsequent G_mapset() calls. It should probably be change to "const char *" to catch this.
comment:19 by , 17 years ago
Replying to glynn:
G_mapset() returns a pointer to a static buffer. If you modify this buffer, you modify the return value for all subsequent G_mapset() calls.
Oops; that's incorrect. G_mapset() actually checks that the value hasn't changed.
follow-up: 21 comment:20 by , 17 years ago
Replying to neteler:
It compares the wrong mapsets here (G_mapset() should possibly not used on the left side, see function remove_subgroup_files() of main.c of i.group.
AFAICT, you should be able to use in=lsat7_2000_61@landsat to acheive the desired result.
Beyond that: unless "landsat" is in the mapset search path, this isn't a bug; you should have to explicitly specify the @landsat part.
OTOH, in the case where it is in the mapset search path, it's a limitation of the imagery library. The I_find_* functions only ever look for files in the current mapset (passing a qualified name to I_find_* will generate an error if the @mapset part doesn't specify the current mapset).
I didn't change this when I fixed the I_find_* functions because I can't easily determine if it will introduce potentially dangerous bugs (e.g. deleting the wrong file).
Unlike the G_find_* functions, the I_find_* functions don't have a mapset argument, so the caller doesn't get the option of passing G_mapset() instead of "" to prevent the use of other mapsets. Bear in mind that the same functions are used for both input (where other mapsets may be used) and output (where only the current mapset may be used).
comment:21 by , 17 years ago
Replying to glynn:
Replying to neteler:
It compares the wrong mapsets here (G_mapset() should possibly not used on the left side, see function remove_subgroup_files() of main.c of i.group.
AFAICT, you should be able to use in=lsat7_2000_61@landsat to acheive the desired result.
this is working but not practical. I could have generated the group some years ago, for a normal user it's hard to find out this trick (yes, use -l and such but...).
Maybe: if a map appears only once in the list, remove it. Otherwise tell the user to indicate the mapset.
Beyond that: unless "landsat" is in the mapset search path, this isn't a bug; you should have to explicitly specify the @landsat part.
It's not obvious (see above).
OTOH, in the case where it is in the mapset search path, it's a limitation of the imagery library. The I_find_* functions only ever look for files in the current mapset (passing a qualified name to I_find_* will generate an error if the @mapset part doesn't specify the current mapset).
AFAIK the imagery library was written in the early days of GRASS, apparently improvements from libgis were never integrated.
I didn't change this when I fixed the I_find_* functions because I can't easily determine if it will introduce potentially dangerous bugs (e.g. deleting the wrong file).
Note that here we just remove a *reference* to a file (though I don't know about side effects of the used functions).
Unlike the G_find_* functions, the I_find_* functions don't have a mapset argument, so the caller doesn't get the option of passing G_mapset() instead of "" to prevent the use of other mapsets. Bear in mind that the same functions are used for both input (where other mapsets may be used) and output (where only the current mapset may be used).
Could the G_find_* functions be used instead?
Markus
comment:22 by , 9 years ago
Milestone: | 6.4.0 → 6.4.6 |
---|
comment:23 by , 6 years ago
Component: | Default → Imagery |
---|---|
CPU: | → Unspecified |
Milestone: | 6.4.6 → 8.0.0 |
Platform: | → Unspecified |
Type: | defect → task |
GRASS 7.8.0 will contain code allowing i.group to list content of groups in other mapsets. It also handles correctly creation of groups with maps from other mapsets (fully qualified map names). It was implemented in r73633 and r74107. See also: #3491
Still other imagery modules are working according to initial design – groups are useable only in the current mapset. Thus a new design should be proposed and modules changed to fit the new approach. I am not certain if such invasive changes should be done in GRASS 7.
One of options would be to handle groups like raster colours – in each mapset an override (shadowing) could be implemented. If a module requests write option to group, files are written into current mapset and thus changes are visible only in the current mapset. Downside – changes are accessible only from current mapset – it could be problematical for sig files.
Other approach would be to take signature files and classification results out from groups completely. There would be no big change in not writing classification output to group. With signature files it is more tricky, as new functions handling sig files then should be introduced or sig file handling should be shifted to the user (output to specified path/file instead of current mapset).
Replying to hamish:
To solve this, i.group needs to be modified to handle the @mapset part correctly. That is the only course of action which can accurately be said to *solve* the issue.
There may be workarounds, but a workaround isn't the same thing as a solution.
And any function named G_ask_<something> shouldn't be used for anything. Those functions cause problems for GUI use (requiring a terminal), and will disappear the moment that 7.x comes into being.