Opened 14 years ago

Closed 14 years ago

Last modified 14 years ago

#1059 closed defect (fixed)

wxGUI: No menu option for r.mapcalc

Reported by: vesnikos Owned by: martinl
Priority: major Milestone: 7.0.0
Component: wxGUI Version: svn-trunk
Keywords: r.mapcalc wxGUI Cc: grass-dev@…
CPU: x86-32 Platform: Linux

Description

I was trying some of the examples of grassbook, and in one of them I needed to use r.mapcalc

I searched it through the command search option using the gui (r.mapcalc) but it didnt return anything. r3.mapcalc exits.

r.mapcalc command is accessible from cli

Change History (22)

comment:1 by martinl, 14 years ago

Cc: grass-dev@… added
Owner: changed from grass-dev@… to martinl
Status: newassigned

r.mapcalc added to the menu in r42183.

comment:2 by vesnikos, 14 years ago

when starting wxgui:

AttributeError: 'GMFrame' object has no attribute 'MapCalculator'

comment:3 by martinl, 14 years ago

Try r42184

comment:4 by vesnikos, 14 years ago

r42184 gave the error

comment:5 by vesnikos, 14 years ago

same error with r42183

in reply to:  description ; comment:6 by mmetz, 14 years ago

Replying to vesnikos:

I was trying some of the examples of grassbook, and in one of them I needed to use r.mapcalc

I searched it through the command search option using the gui (r.mapcalc) but it didnt return anything. r3.mapcalc exits.

r.mapcalc command is accessible from cli

I think the menu entry "Map calculator" between "Mask" and "Neighborhood Analysis" is what you are looking for, present in r42182 and earlier.

The wxGUI of r42183 and r42184 is broken for me too, same error here as in comment 2.

Markus M

comment:7 by vesnikos, 14 years ago

thank you markus, thats what i was looking for.

Still shouldn't r.mapcalc open the said window?

in reply to:  6 comment:8 by martinl, 14 years ago

Replying to mmetz:

The wxGUI of r42183 and r42184 is broken for me too, same error here as in comment 2.

r42187 ?

comment:9 by vesnikos, 14 years ago

works like charm. No crashes, starts up. good fix

comment:10 by martinl, 14 years ago

Resolution: fixed
Status: assignedclosed

in reply to:  10 ; comment:11 by mmetz, 14 years ago

Resolution: fixed
Status: closedreopened

I am not so sure about these changes. In 6.4.0RC6 I can do

GRASS 6.4.0RC6 (nc_spm_08):~ > r.mapcalc
Enter expressions, "end" when done.
mapcalc> test = 1.1
mapcalc> test2 = 2.1
mapcalc> end

 100%
GRASS 6.4.0RC6 (nc_spm_08):~ >

This is now no longer possible in grass7. In grass6 there is r.mapcalculator which pops up a GUI dialog and is a wrapper for r.mapcalc. r.mapcalculator is present in grass7, but as bash script, apparently it awaits translation to python? I personally would rather like to have r.mapcalc in trunk as in r42182 and use r.mapcalculator to provide a GUI interface like is done now for the modified r.mapcalc, another one than the fancy Map calculator? All the recent changes are done only to get a hit when searching the module in the wxGUI? IMHO, functionality has been removed, not added.

Reopening the ticket for discussion.

Markus M

in reply to:  11 ; comment:12 by glynn, 14 years ago

Replying to mmetz:

This is now no longer possible in grass7.

r42183 was incomplete; this should be fixed by r42210.

r.mapcalculator is present in grass7, but as bash script, apparently it awaits translation to python?

The consensus was that it wasn't worth maintaining. In 7.0, r.mapcalc uses G_parser(), so it can be run directly from the GUI without requiring a wrapper.

All the recent changes are done only to get a hit when searching the module in the wxGUI?

r42183 had two parts: fixing the GUI menu, and cosmetic changes to r.mapcalc. The latter included renaming the input= option to file=, but overlooked the fact that it synthesises "input=-" when run without arguments (to preserve the historical behaviour). The change broke this behaviour; r42210 reinstates it.

in reply to:  12 ; comment:13 by mmetz, 14 years ago

Replying to glynn:

Replying to mmetz:

This is now no longer possible in grass7.

r42183 was incomplete; this should be fixed by r42210.

Thanks!

r.mapcalculator is present in grass7, but as bash script, apparently it awaits translation to python?

The consensus was that it wasn't worth maintaining. In 7.0, r.mapcalc uses G_parser(), so it can be run directly from the GUI without requiring a wrapper.

OK, nice.

All the recent changes are done only to get a hit when searching the module in the wxGUI?

r42183 had two parts: fixing the GUI menu, and cosmetic changes to r.mapcalc. The latter included renaming the input= option to file=, but overlooked the fact that it synthesises "input=-" when run without arguments (to preserve the historical behaviour). The change broke this behaviour; r42210 reinstates it.

Why not just

Index: gui/wxpython/xml/menudata.xml
===================================================================
--- gui/wxpython/xml/menudata.xml	(revision 42182)
+++ gui/wxpython/xml/menudata.xml	(working copy)
@@ -1010,7 +1010,9 @@
 	<menuitem>
 	  <label>Map calculator</label>
 	  <help>Map calculator for raster map algebra</help>
+	  <keywords>raster</keywords>
 	  <handler>DispMapCalculator</handler>
+	  <command>r.mapcalc</command>
 	</menuitem>
 	<menu>
 	  <label>Neighborhood analysis</label>

against r42182? AFAICT, no modifications needed for raster/r.mapcalc

BTW, could the Map calculator get an output window and the r.mapcalc manual?

Thanks,

Markus M

in reply to:  13 ; comment:14 by martinl, 14 years ago

Replying to mmetz:

> Index: gui/wxpython/xml/menudata.xml
> ===================================================================
> --- gui/wxpython/xml/menudata.xml	(revision 42182)
> +++ gui/wxpython/xml/menudata.xml	(working copy)
> @@ -1010,7 +1010,9 @@
>  	<menuitem>
>  	  <label>Map calculator</label>
>  	  <help>Map calculator for raster map algebra</help>
> +	  <keywords>raster</keywords>
>  	  <handler>DispMapCalculator</handler>
> +	  <command>r.mapcalc</command>
>  	</menuitem>
>  	<menu>
>  	  <label>Neighborhood analysis</label>
> 

It shows wxGUI Map Calculator. There is already item for r.mapcalc in the menu grass/trunk/gui/wxpython/xml/menudata.xml@42210#L1011

in reply to:  13 comment:15 by martinl, 14 years ago

Replying to mmetz:

r.mapcalculator is present in grass7, but as bash script, apparently it awaits translation to python?

I think we don't need r.mapcalculator, r.mapcalc uses G_parser() and moreover there is wxGUI Map Calculator also available. I would suggest to remove this script from trunk.

in reply to:  14 ; comment:16 by mmetz, 14 years ago

Replying to martinl:

Replying to mmetz:

> Index: gui/wxpython/xml/menudata.xml
> ===================================================================
> --- gui/wxpython/xml/menudata.xml	(revision 42182)
> +++ gui/wxpython/xml/menudata.xml	(working copy)
> @@ -1010,7 +1010,9 @@
>  	<menuitem>
>  	  <label>Map calculator</label>
>  	  <help>Map calculator for raster map algebra</help>
> +	  <keywords>raster</keywords>
>  	  <handler>DispMapCalculator</handler>
> +	  <command>r.mapcalc</command>
>  	</menuitem>
>  	<menu>
>  	  <label>Neighborhood analysis</label>
> 

It shows wxGUI Map Calculator. There is already item for r.mapcalc in the menu grass/trunk/gui/wxpython/xml/menudata.xml@42210#L1011

I was talking about r42182, before this flood of changes came in. My diff is against revision 42182, not revision 42210. In r42210, a lot of changes have been made to the GUI and the module, some of the changes have been changed again...

My point is that probably much less changes were required to make r.mapcalc visible in the wxGUI command search, that the GUI should be fixed first, before modules get broken by a quick fix, that the two different interfaces for r.mapcalc are confusing, that more testing should be done on the local copy before a commit, and that the Map Calculator wxGUI interface is nice but unfinished work, no output, no help. I personally would recommend to go back to r42182 and start anew, looking for a solution that requires less modifications.

Sorry for the ranting, but even though trunk is the development version, I would sometimes prefer some more testing before submitting changes, and a flood of commits for the same problem within one day clearly indicates that the first commits were not well thought through.

Markus M

in reply to:  16 ; comment:17 by martinl, 14 years ago

Replying to mmetz:

I was talking about r42182, before this flood of changes came in. My diff is against revision 42182, not revision 42210. In r42210, a lot of changes have been made to the GUI and the module, some of the changes have been changed again...

Most of the changes are related to the major clean up of mcalc_builder.py which is unrelated to this ticket. It was just long-term TODO.

My point is that probably much less changes were required to make r.mapcalc visible in the wxGUI command search, that the GUI should be fixed first, before modules get broken by a quick

Seems to work for me, http://gama.fsv.cvut.cz/~landa/grass/swf/wxgui-search-module.html. r.mapcalc has been added to the menu by changing menudata.xml, nothing more. Anyway sorry that I broke the r.mapcalc interface.

fix, that the two different interfaces for r.mapcalc are confusing, that more testing should be done on the local copy before a commit, and that the Map Calculator wxGUI interface is nice but unfinished work, no output, no help. I personally would recommend to go back to r42182 and start anew, looking for a solution that requires less modifications.

I am not original author of wxGUI Map Calc. In any case it's a usable tool. Of course it needs various fixes, any volunteer welcomed... See simple example at http://gama.fsv.cvut.cz/~landa/grass/swf/wxgui-mapcalc.html. Anyway to have two items in the menu can be confusing.

Sorry for the ranting, but even though trunk is the development version, I would sometimes prefer some more testing before submitting changes, and a flood of commits for the same problem within one day clearly indicates that the first commits were not well thought through.

wxGUI is quite experimental from my point of view. It's not part of 'core', anyway this flood of commits refers mostly to clean up of mcalc_bulder.py module.

in reply to:  17 ; comment:18 by mmetz, 14 years ago

Replying to martinl:

Replying to mmetz:

I was talking about r42182, before this flood of changes came in. My diff is against revision 42182, not revision 42210. In r42210, a lot of changes have been made to the GUI and the module, some of the changes have been changed again...

Most of the changes are related to the major clean up of mcalc_builder.py which is unrelated to this ticket. It was just long-term TODO.

There were at least 4 commits related to this ticket, these I mean. IMHO, the first two were done a bit in a hurry, and I want to ask for some more care and testing before committing (nice word in this context), granted that some bugs can always slip through.

[snip], that the two different interfaces for r.mapcalc are confusing.

[snip] Anyway to have two items in the menu can be confusing.

So is there really a need for two GUI interfaces for r.mapcalc? I'm sure it is possible to link r.mapcalc to the Map calculator GUI in the GUI command search.

wxGUI is quite experimental from my point of view. It's not part of 'core',

From this point of view, even less reason to modify a core module like r.mapcalc just for a quick wxGUI fix. OTOH, it's the only GUI in trunk, and default for 6.4+ IIRC...

comment:19 by cmbarton, 14 years ago

If you type a GRASS command and press return, the parser reads xml information pass from running [command] --interface-description, and builds a basic GUI from this. Actually, the module GUI it builds is pretty sophisticated, with format control, choice boxes, color pickers, etc. If you run r.mapcalc this way, this is the kind of GUI you get--one that gives you a more accessible way to complete required and optional arguments, see output, and have access to the manual.

The map calculator that is accessible from the GUI is a special module that only runs in the wxPython environment of the GUI. It presents a much more graphical interface, with a "calculator" like appearance. It is not callable automatically by the parser when you run r.mapcalc.

To link this calculator-like interface to mapcalc, you probably would need to change the parser code to make a special exception for r.mapcalc. Then you would need to change the map calculator code to make it run stand-alone. Neither of these changes are trivial.

And I don't think this is really necessary. Most people running r.mapcalc from the command line will be comfortable with entering mapcalc expressions. Those running the map calculator can use expressions, but don't need to. The calculator can help them learn these expressions, however. Nonetheless, they are likely to be a different set of users.

Michael

in reply to:  18 ; comment:20 by martinl, 14 years ago

Replying to mmetz:

I was talking about r42182, before this flood of changes came in. My diff is against revision 42182, not revision 42210. In r42210, a lot of changes have been made to the GUI and the module, some of the changes have been changed again...

Most of the changes are related to the major clean up of mcalc_builder.py which is unrelated to this ticket. It was just long-term TODO.

There were at least 4 commits related to this ticket, these I mean. IMHO, the first two were done a bit in a hurry, and I want to ask for some more care and testing before committing (nice word in this context), granted that some bugs can always slip through.

"Hurry" is a relative word ;-)

[snip], that the two different interfaces for r.mapcalc are confusing.

[snip] Anyway to have two items in the menu can be confusing.

So is there really a need for two GUI interfaces for r.mapcalc? I'm sure it is possible to link r.mapcalc to the Map calculator GUI in the GUI command search.

No, r.mapcalc would searchable even not included in the menu. The owner of this ticket requested to add r.mapcalc to the menu. So I did it.

From this point of view, even less reason to modify a core module like r.mapcalc just for a quick wxGUI fix. OTOH, it's the only GUI in trunk, and default for 6.4+ IIRC...

Sorry, as a main contributor to the wxGUI I have little time for testing. Moreover trunk is used for development. Broken r.mapcalc has been fixed very soon thanks to Glynn. So where is the problem?

in reply to:  20 ; comment:21 by mmetz, 14 years ago

Resolution: fixed
Status: reopenedclosed

Replying to martinl:

Replying to mmetz:

So is there really a need for two GUI interfaces for r.mapcalc? I'm sure it is possible to link r.mapcalc to the Map calculator GUI in the GUI command search.

No, r.mapcalc would searchable even not included in the menu. The owner of this ticket requested to add r.mapcalc to the menu. So I did it.

See comment:6 and comment:7

The owner of the ticket wondered why the Map Culculator did not show up when searching for r.mapcalc in the wxGUI command search option.

When I search for r.colors, I get 4 hits, so it is obviously possible in the wxGUI command search to link a given command to more than one GUI dialog and to link it to a custom dialog that is not generated automatically by parsing [command] --interface-description. The interactive map calculator is a GUI front end for r.mapcalc, so it would be nice if that shows up when searching for r.mapcalc in the wxGUI command search option.

From this point of view, even less reason to modify a core module like r.mapcalc just for a quick wxGUI fix. OTOH, it's the only GUI in trunk, and default for 6.4+ IIRC...

Sorry, as a main contributor to the wxGUI I have little time for testing. Moreover trunk is used for development. Broken r.mapcalc has been fixed very soon thanks to Glynn. So where is the problem?

IMHO, as a main contributor to the wxGUI, you should spend most of the time testing because it is still quite experimental to quote you.

My problem is that I don't like quick and dirty fixes and that I think it's a good idea if main contributors test changes first before committing so as not to break things.

Closing ticket because something shows up now when searching for the command in question in the wxGUI command search and because this discussion is no longer really related to the ticket, it's just my personal nagging about (more) testing and being more careful when trying to fix something.

in reply to:  21 comment:22 by martinl, 14 years ago

Replying to mmetz:

The owner of the ticket wondered why the Map Culculator did not show up when searching for r.mapcalc in the wxGUI command search option.

...and also see the title of the ticket...

Note: See TracTickets for help on using tickets.