Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[bugfix] [saga] Fix saga_cmd command with multiple outputs #33680

Merged
merged 1 commit into from Jan 13, 2020
Merged

[bugfix] [saga] Fix saga_cmd command with multiple outputs #33680

merged 1 commit into from Jan 13, 2020

Conversation

agiudiceandrea
Copy link
Contributor

Description

Fixes a regression bug accidentally introduced with 40134d6 (PR #8968) since 3.6.0 and backported to 3.4 branch with a887b7d (PR #9231) since 3.4.6 : when there are multiple output parameters, processAlgorithm incorrectly generates multiple saga_cmd commands, instead of a single command containing the output parameters.

It should be backported to 3.10 branch.

Fixes #33658

Checklist

  • Commit messages are descriptive and explain the rationale for changes
  • Commits which fix bugs include Fixes #11111 at the bottom of the commit message
  • I have read the QGIS Coding Standards and this PR complies with them
  • New unit tests have been added for core changes
  • I have run the scripts/prepare-commit.sh script before each commit
  • I have evaluated whether it is appropriate for this PR to be backported, backport requests are left as label or comment

@agiudiceandrea
[SAGA] Fix saga_cmd command with multiple outputs
1a6fbb0 
Fixes a regression bug accidentally introduced with 40134d6 (PR #8968) and backported with a887b7d (PR #9231): when there are multiple output parameters processAlgorithm incorrectly generates multiple saga_cmd commands, instead of a single command containing the output parameters.

Fixes #33658
@luipir
Copy link
Contributor

luipir commented Jan 9, 2020

@agiudiceandrea is it too much to add test running the alg without temporary output?

the reason is that this error would be spotted out if in https://github.com/qgis/QGIS/pull/8968/files#diff-6f9c9744d48eaf681189d8318635dc35R99 would have also a for temporary output

btw If you do not have time to add, I'm ok to merge anyway

alexbruy
alexbruy approved these changes Jan 9, 2020
View reviewed changes
Copy link
Contributor

@alexbruy alexbruy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@alexbruy alexbruy added backport release-3_10 Bug Either a bug report, or a bug fix. Let's hope for the latter! Processing Relating to QGIS Processing framework or individual Processing algorithms labels Jan 9, 2020
@github-actions github-actions bot added this to the 3.12.0 milestone Jan 10, 2020
@nyalldawson nyalldawson merged commit 0d4d825 into qgis:master Jan 13, 2020
@qgis-bot qgis-bot mentioned this pull request Jan 13, 2020
Merged
@agiudiceandrea agiudiceandrea deleted the patch-1 branch January 22, 2020 12:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexbruy alexbruy alexbruy approved these changes

Assignees
No one assigned
Labels
Bug Either a bug report, or a bug fix. Let's hope for the latter! Processing Relating to QGIS Processing framework or individual Processing algorithms
Projects
None yet
Milestone
3.12.0
Development

Successfully merging this pull request may close these issues.

SAGA SagaAlgorithm().processAlgorithm() builds multiple saga_cmd commands for composite algorithms
4 participants
@agiudiceandrea @luipir @alexbruy @nyalldawson