Opened 10 years ago

Closed 10 years ago

#2414 closed enhancement (fixed)

Replace g.list/g.remove with g.mlist/g.mremove

Reported by: hcho Owned by: hcho
Priority: blocker Milestone: 7.0.0
Component: Default Version: svn-trunk
Keywords: g.list, g.remove, g.mlist, g.mremove Cc:
CPU: All Platform: All

Description

I propose to replace the old g.list and g.remove modules with enhanced g.mlist and g.mremove. This will break backward compatibility because of changes in API and output behavior.

By default, g.mlist prints outputs in a machine friendly format (e.g., not directed to a paper) while g.list prints in human readable format using a pager. g.mlist -p does pretty printing. g.mlist additionally has pattern=, exclude=, separator=, region=, and output= options.

g.mremove has only one option for specifying data types to delete while g.remove has as many options as available data types. Since the first option in g.remove is rast=, the user can delete raster maps by mistake when s/he actually intended to delete vector maps. With g.mremove, type= is required and the user must specify data types explicitly. g.mremove additionally has pattern= (for map names or search pattern) and exclude= options.

Please let me know if you have good reasons to keep the old g.list and g.remove. Otherwise, I suggest to replace g.list and g.remove with g.mlist and g.mremove before 7.0 release.

Change History (14)

in reply to:  description ; comment:1 by wenzeslaus, 10 years ago

Keywords: g.mlist g.mremove added

Replying to hcho:

I propose to replace the old g.list and g.remove modules with enhanced g.mlist and g.mremove. This will break backward compatibility because of changes in API and output behavior.

I agree, the old modules are not necessary.

By default, g.mlist prints outputs in a machine friendly format (e.g., not directed to a paper) while g.list prints in human readable format using a pager. g.mlist -p does pretty printing.

I'm not sure what is the right default. It seems to me that -p should be the pretty print, so this gives me that without -p it should do what g.mlist is doing by default, i.e. the interface should be the one of g.mlist.

g.mremove has only one option for specifying data types to delete while g.remove has as many options as available data types. Since the first option in g.remove is rast=, the user can delete raster maps by mistake when s/he actually intended to delete vector maps. With g.mremove, type= is required and the user must specify data types explicitly.

This will close #1259.

in reply to:  1 comment:2 by martinl, 10 years ago

Replying to wenzeslaus:

Replying to hcho:

I propose to replace the old g.list and g.remove modules with enhanced g.mlist and g.mremove. This will break backward compatibility because of changes in API and output behavior.

I agree, the old modules are not necessary.

+1

comment:3 by hcho, 10 years ago

I'll rename g.mlist/g.mremove to g.list/g.remove in the next few days.

in reply to:  3 ; comment:4 by hcho, 10 years ago

Replying to hcho:

I'll rename g.mlist/g.mremove to g.list/g.remove in the next few days.

BTW, what is the best way to do this with SVN? I believe that we want to keep the history of all the four modules while deleting and renaming them. I need help from an SVN expert.

in reply to:  4 comment:5 by wenzeslaus, 10 years ago

Replying to hcho:

Replying to hcho:

I'll rename g.mlist/g.mremove to g.list/g.remove in the next few days.

BTW, what is the best way to do this with SVN? I believe that we want to keep the history of all the four modules while deleting and renaming them. I need help from an SVN expert.

When I was removing r.los, I actually moved it to addons (for 7). However, g.list and g.remove are different since they are not alternative implementation and the different API is not so interesting since at least g.list was probably not much used in user scripts (while r.los was and with the same name, it can be downloaded and used if necessary).

So, I would say that the right way is to remove g.list and g.remove directories and then move/rename g.mlist and g.mremove. I think there are some tests in g.list and g.remove. For simplicity, I would ignore them (delete together with modules) and then add new files with the old content (which will be necessary to change according to new API).

(BTW, if I remember correctly, Git, unlike SVN, usually recognizes the same files by itself, so removing the file and adding a similar file somewhere else in one commit results in a link between files.)

comment:6 by hcho, 10 years ago

I deleted g.list/g.remove and renamed g.mlist/g.mremove to g.list/g.remove (r62009, r62010, r62011, r62013). I'm updating the python/shell scripts that use these modules. The interface of g.remove has changed as follows:

g.remove rast=map => g.remove -f type=rast pattern=map
g.remove map => g.remove -f rast p=map

Note the -f flag. It's more typing including pattern=.

comment:7 by hcho, 10 years ago

All text files referring to g.mlist and g.mremove have been updated in r62015. This revision is quite a big one, so we need to test it before back porting to rel70.

comment:8 by wenzeslaus, 10 years ago

Most of the tests are still running (the exception is r3.gradient):

However, the test coverage is questionable regarding g.list and g.remove, so tests by users would be really good (both GUI and script).

Just as a remainder, we should not forget about addons when backporting.

comment:9 by hcho, 10 years ago

r3.gradient fixed in r62020.

comment:10 by martinl, 10 years ago

GRASS 7.0 still contains g.mlist and g.mremove, probably time for backport?

in reply to:  10 comment:11 by hcho, 10 years ago

Replying to martinl:

GRASS 7.0 still contains g.mlist and g.mremove, probably time for backport?

I agree. I'll test the latest g.list/g.remove in trunk including GUI and backport them this week.

comment:12 by neteler, 10 years ago

Please consider to backport the g.list/g.remove related changes.

in reply to:  12 comment:13 by hcho, 10 years ago

Replying to neteler:

Please consider to backport the g.list/g.remove related changes.

Both modules are backported in r62393 and r62396.

comment:14 by neteler, 10 years ago

Resolution: fixed
Status: newclosed

Closing as solved.

Note: See TracTickets for help on using tickets.