add GRASS r.mapcalculator, remove r.mapcalc #8444

gioman · 2018-11-08T17:39:46Z

Description

GRASS "r.mapcalc" is a beast that because of its nature was always hard to try implement in QGIS/Processing, and in fact in NEVER worked
"r.mapcalculator" is a GRASS python script that makes use of "r.mapcalc" and makes it possible to implement it in QGIS/Processing like almost any other GRASS tool
We had lost "r.mapcalcultor" in QGIS 3 because this was not ported from GRASS64 to GRASS7, but now a very nice GRASS developer made the porting, so we will have again access to this very powerful tool

https://trac.osgeo.org/grass/ticket/3431#comment:7

This should be merged once the ported script will make its own way into the main branch of GRASS7

Checklist

Reviewing is a process done by project maintainers, mostly on a volunteer basis. We try to keep the overhead as small as possible and appreciate if you help us to do so by completing the following items. Feel free to ask in a comment if you have troubles with any of them.

Commit messages are descriptive and explain the rationale for changes
Commits which fix bugs include fixes #11111 in the commit message next to the description
Commits which add new features are tagged with [FEATURE] in the commit message
Commits which change the UI or existing user workflows are tagged with [needs-docs] in the commit message and contain sufficient information in the commit message to be documented
I have read the QGIS Coding Standards and this PR complies with them
This PR passes all existing unit tests (test results will be reported by travis-ci after opening this PR)
New unit tests have been added for core changes
I have run the scripts/prepare-commit.sh script before each commit

nyalldawson · 2018-11-11T19:33:21Z

GRASS "r.mapcalc" is a beast that because of its nature was always hard to try implement in QGIS/Processing, and in fact in NEVER worke

Is this a "NEVER EVER EVER" worked? Is there any chance a user could have a working model broken by dropping this?

gioman · 2018-11-12T12:48:22Z

Is this a "NEVER EVER EVER" worked? Is there any chance a user could have a working model broken by dropping this?

@nyalldawson I always followed closely any change in this module and personally I found it never worked (Windows or Linux).

PedroVenancio · 2018-11-12T15:05:12Z

@nyalldawson

I confirm that despite the attempts done to make GRASS7 r.mapcalc work on Processing, it was never possible to completely overcome the issues described in the feature request / bug report: https://issues.qgis.org/issues/6894

Now with the port of r.mapcalcultor, it seems to be working very well!

nyalldawson · 2018-11-12T21:18:07Z

Ok, cool. Just scared about breaking stuff if it was working for some users previously. Could we get a unit test here too please?

gioman · 2018-11-13T11:12:27Z

Could we get a unit test here too please?

yes of course. Anyway this is not to merge yet as the script has not yet been merged in GRASS main tree.

stale · 2018-11-27T12:50:20Z

The QGIS project highly values your contribution and would love to see this work merged! Unfortunately this PR has not had any activity in the last 14 days and is being automatically marked as "stale". If you think this pull request should be merged, please check

that all unit tests are passing
that all comments by reviewers have been addressed
that there is enough information for reviewers, in particular
- link to any issues which this pull request fixes
- add a description of workflows which this pull request fixes
- add screenshots if applicable
that you have written unit tests where possible
In case you should have any uncertainty, please leave a comment and we will be happy to help you proceed with this pull request.
If there is no further activity on this pull request, it will be closed in a week.

PedroVenancio · 2018-11-27T13:11:47Z

This needs to wait for this change:

https://trac.osgeo.org/grass/ticket/3431#comment:9

gioman · 2018-12-11T12:42:43Z

Note to myself. The tool will land in GRASS 7.6 and will called "r.mapcalc.simple", so this PR must be updated.

stale · 2018-12-25T16:15:02Z

The QGIS project highly values your contribution and would love to see this work merged! Unfortunately this PR has not had any activity in the last 14 days and is being automatically marked as "stale". If you think this pull request should be merged, please check

that all unit tests are passing
that all comments by reviewers have been addressed
that there is enough information for reviewers, in particular
- link to any issues which this pull request fixes
- add a description of workflows which this pull request fixes
- add screenshots if applicable
that you have written unit tests where possible
In case you should have any uncertainty, please leave a comment and we will be happy to help you proceed with this pull request.
If there is no further activity on this pull request, it will be closed in a week.

neteler · 2018-12-27T12:01:31Z

Note, it has been backported to GRASS GIS 7.4 (to be part of upcoming 7.4.4) as well besides GRASS GIS 7.6 (upcoming 7.6.0):

gioman · 2018-12-28T14:08:43Z

@neteler very good news indeed, thanks!

PedroVenancio · 2018-12-28T14:59:15Z

@gioman @nyalldawson

Can we ensure that if GRASS 7.4.4 came out before 2019-01-18 (release date of QGIS 2.18.28), the Processing r.mapcalc.simple will be backported to be part of QGIS 2.18.28?

We should take advantage of this GRASS developers effort to include r.mapcalc.simple in the latest version of QGIS 2.18 LTR:

https://trac.osgeo.org/grass/ticket/3431#comment:27

The effort on the part of QGIS is minimal and the possibility of introducing bugs is nil.

On the other hand, it is also necessary to ensure that QGIS 2.18.28 is distributed with GRASS 7.4.4 on OSGeo4W and standalone for Windows.

What do you think?

gioman · 2018-12-28T15:16:06Z

We should take advantage of this GRASS developers effort to include r.mapcalc.simple in the latest version of QGIS 2.18 LTR:

agree

On the other hand, it is also necessary to ensure that QGIS 2.18.28 is distributed with GRASS 7.4.4 on OSGeo4W and standalone for Windows.

for this maybe also better include @jef-n in this discussion.

neteler · 2018-12-28T17:50:23Z

@gioman @nyalldawson

Can we ensure that if GRASS 7.4.4 came out before 2019-01-18 (release date of QGIS 2.18.28), the Processing r.mapcalc.simple will be backported to be part of QGIS 2.18.28?

We shall be able to make the GRASS GIS 7.4.4 release happen by then:
https://trac.osgeo.org/grass/milestone/7.4.4

Edit: Now proposed:
https://lists.osgeo.org/pipermail/grass-dev/2018-December/090810.html

neteler · 2019-01-04T19:38:33Z

@gioman @nyalldawson
Can we ensure that if GRASS 7.4.4 came out before 2019-01-18 (release date of QGIS 2.18.28), the Processing r.mapcalc.simple will be backported to be part of QGIS 2.18.28?

We shall be able to make the GRASS GIS 7.4.4 release happen by then:
https://trac.osgeo.org/grass/milestone/7.4.4

Done: Just now GRASS GIS 7.4.4 has been published:
https://trac.osgeo.org/grass/wiki/Release/7.4.4-News

along with
https://grass.osgeo.org/grass74/manuals/r.mapcalc.simple.html

gioman · 2019-01-07T10:30:02Z

https://trac.osgeo.org/grass/milestone/7.4.4

Done: Just now GRASS GIS 7.4.4 has been published:
https://trac.osgeo.org/grass/wiki/Release/7.4.4-News

along with
https://grass.osgeo.org/grass74/manuals/r.mapcalc.simple.html

@neteler thanks! I backported the processing patch to 2.18 and 3.4 branches.

luipir · 2019-01-07T10:44:51Z

@gioman can you find a way to check the installed grass version to avoid to have this command if grass is less that 7.4

neteler · 2019-01-07T10:46:55Z

@gioman can you find a way to check the installed grass version to avoid to have this command if grass is less that 7.4

grass --config version
7.4.4

gioman · 2019-01-07T10:49:37Z

@gioman can you find a way to check the installed grass version to avoid to have this command if grass is less that 7.4

I guess is possible, but I can't do it right now. I think is more important to have this merged anyway, as anyway we are shipping QGIS since long with a broken (it never worked) r.mapcalc module. Who will not have GRASS 7.4.4 will have a not working r.mapcalc.simple module, that do not change much in term of ux :)

luipir · 2019-01-07T12:33:26Z

@gioman will you add tests during A Coruña hackmeeting?

gioman · 2019-01-07T12:38:03Z

@gioman will you add tests during A Coruña hackmeeting?

yes

gioman added 2 commits November 8, 2018 17:35

add GRASS r.mapcalculator, remove r.mapcalc

a866d49

add missing file

d286598

nyalldawson added the Feature label Nov 11, 2018

nyalldawson added this to the 3.6 milestone Nov 11, 2018

stale bot added the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Nov 27, 2018

stale bot removed the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Nov 27, 2018

added missing *

8e223e6

stale bot added the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Dec 25, 2018

stale bot removed the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Dec 28, 2018

gioman added 2 commits January 7, 2019 09:54

change the name of the module to match the new name in GRASS 7.4.4

667a86b

add missed file

9f3c313

gioman mentioned this pull request Jan 7, 2019

re-add the r.mapcalculator module (as r.mapcalc.simple) and remove the not working r.mapcalc #8802

Merged

8 tasks

fix the module name

10d4799

gioman mentioned this pull request Jan 7, 2019

re-add the r.mapcalculator momodule (as r.mapcalc.simple) and remove the not working r.mapcalc #8803

Merged

8 tasks

fix module description file

a57fec8

luipir approved these changes Jan 7, 2019

View reviewed changes

luipir merged commit 83c6a2c into qgis:master Jan 7, 2019

This was referenced May 24, 2019

GRASS r.mapcalc cumbersome to use, issues on Windows #16019

Open

Problem with GRASS 7 algorithms that call GRASS scripts under Windows #28322

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add GRASS r.mapcalculator, remove r.mapcalc #8444

add GRASS r.mapcalculator, remove r.mapcalc #8444

gioman commented Nov 8, 2018 •

edited

Loading

nyalldawson commented Nov 11, 2018

gioman commented Nov 12, 2018

PedroVenancio commented Nov 12, 2018

nyalldawson commented Nov 12, 2018

gioman commented Nov 13, 2018

stale bot commented Nov 27, 2018

PedroVenancio commented Nov 27, 2018

gioman commented Dec 11, 2018

stale bot commented Dec 25, 2018

neteler commented Dec 27, 2018

gioman commented Dec 28, 2018

PedroVenancio commented Dec 28, 2018

gioman commented Dec 28, 2018

neteler commented Dec 28, 2018 •

edited

Loading

neteler commented Jan 4, 2019

gioman commented Jan 7, 2019

luipir commented Jan 7, 2019

neteler commented Jan 7, 2019

gioman commented Jan 7, 2019

luipir commented Jan 7, 2019

gioman commented Jan 7, 2019

add GRASS r.mapcalculator, remove r.mapcalc #8444

add GRASS r.mapcalculator, remove r.mapcalc #8444

Conversation

gioman commented Nov 8, 2018 • edited Loading

Description

Checklist

nyalldawson commented Nov 11, 2018

gioman commented Nov 12, 2018

PedroVenancio commented Nov 12, 2018

nyalldawson commented Nov 12, 2018

gioman commented Nov 13, 2018

stale bot commented Nov 27, 2018

PedroVenancio commented Nov 27, 2018

gioman commented Dec 11, 2018

stale bot commented Dec 25, 2018

neteler commented Dec 27, 2018

gioman commented Dec 28, 2018

PedroVenancio commented Dec 28, 2018

gioman commented Dec 28, 2018

neteler commented Dec 28, 2018 • edited Loading

neteler commented Jan 4, 2019

gioman commented Jan 7, 2019

luipir commented Jan 7, 2019

neteler commented Jan 7, 2019

gioman commented Jan 7, 2019

luipir commented Jan 7, 2019

gioman commented Jan 7, 2019

gioman commented Nov 8, 2018 •

edited

Loading

neteler commented Dec 28, 2018 •

edited

Loading