Skip to content

Commit 7f7c7a9

Browse files
nyalldawsonnyalldawson
committedJan 29, 2019
[processing][needs-docs] Add friendlier API for running algorithms as sub-steps
of main algorithm Using code like: buffered_layer = processing.run(..., context, feedback)['OUTPUT'] ... return {'OUTPUT': buffered_layer} can cause issues if done as a sub-step of a larger processing algorithm. This is because ownership of the generated layer is transferred to the caller (Python) by processing.run. When the algorithm returns, Processing attempts to move ownership of the layer from the context to the caller, resulting in a crash. (This is by design, because processing.run has been optimised for the most common use case, which is one-off execution of algorithms as part of a script, not as part of another processing algorithm. Accordingly by design it returns layers and ownership to the caller, making things easier for callers as they do not then have to resolve the layer reference from the context object and handle ownership themselves) This commit adds a new "is_child_algorithm" argument to processing.run. For algorithms which are executed as sub-steps of a larger algorithm is_child_algorithm should be set to True to avoid any ownership issues with layers. E.g. buffered_layer = processing.run(..., context, feedback, is_child_algorithm=True)['OUTPUT'] ... return {'OUTPUT': buffered_layer}
1 parent 82ec141 commit 7f7c7a9

File tree

3 files changed

+118
-4
lines changed

3 files changed

+118
-4
lines changed
 

‎python/plugins/processing/tests/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ PLUGIN_INSTALL(processing tests/testdata ${TEST_DATA_FILES})
66

77
IF(ENABLE_TESTS)
88
INCLUDE(UsePythonTest)
9+
ADD_PYTHON_TEST(ProcessingGeneralTest ProcessingGeneralTest.py)
910
ADD_PYTHON_TEST(ProcessingGuiTest GuiTest.py)
1011
ADD_PYTHON_TEST(ProcessingModelerTest ModelerTest.py)
1112
ADD_PYTHON_TEST(ProcessingProjectProviderTest ProjectProvider.py)

‎python/plugins/processing/tests/ProcessingGeneralTest.py

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
# -*- coding: utf-8 -*-
2+
3+
"""
4+
***************************************************************************
5+
QgisAlgorithmTests.py
6+
---------------------
7+
Date : January 2019
8+
Copyright : (C) 2019 by Nyall Dawson
9+
Email : nyall dot dawson at gmail dot com
10+
***************************************************************************
11+
* *
12+
* This program is free software; you can redistribute it and/or modify *
13+
* it under the terms of the GNU General Public License as published by *
14+
* the Free Software Foundation; either version 2 of the License, or *
15+
* (at your option) any later version. *
16+
* *
17+
***************************************************************************
18+
"""
19+
20+
__author__ = 'Nyall Dawson'
21+
__date__ = 'January 2019'
22+
__copyright__ = '(C) 2019, Nyall Dawson'
23+
24+
# This will get replaced with a git SHA1 when you do a git archive
25+
26+
__revision__ = ':%H$'
27+
28+
import nose2
29+
import shutil
30+
import gc
31+
32+
from qgis.core import (QgsApplication,
33+
QgsProcessing,
34+
QgsProcessingContext,
35+
QgsVectorLayer)
36+
from qgis.PyQt import sip
37+
from qgis.analysis import (QgsNativeAlgorithms)
38+
from qgis.testing import start_app, unittest
39+
import processing
40+
from processing.tests.TestData import points
41+
42+
43+
class TestProcessingGeneral(unittest.TestCase):
44+
45+
@classmethod
46+
def setUpClass(cls):
47+
start_app()
48+
from processing.core.Processing import Processing
49+
Processing.initialize()
50+
QgsApplication.processingRegistry().addProvider(QgsNativeAlgorithms())
51+
cls.cleanup_paths = []
52+
cls.in_place_layers = {}
53+
cls.vector_layer_params = {}
54+
55+
@classmethod
56+
def tearDownClass(cls):
57+
from processing.core.Processing import Processing
58+
Processing.deinitialize()
59+
for path in cls.cleanup_paths:
60+
shutil.rmtree(path)
61+
62+
def testRun(self):
63+
context = QgsProcessingContext()
64+
65+
# try running an alg using processing.run - ownership of result layer should be transferred back to the caller
66+
res = processing.run('qgis:buffer',
67+
{'DISTANCE': 1, 'INPUT': points(), 'OUTPUT': QgsProcessing.TEMPORARY_OUTPUT},
68+
context=context)
69+
self.assertIn('OUTPUT', res)
70+
# output should be the layer instance itself
71+
self.assertIsInstance(res['OUTPUT'], QgsVectorLayer)
72+
# Python should have ownership
73+
self.assertTrue(sip.ispyowned(res['OUTPUT']))
74+
del context
75+
gc.collect()
76+
self.assertFalse(sip.isdeleted(res['OUTPUT']))
77+
78+
# now try using processing.run with is_child_algorithm = True. Ownership should remain with the context
79+
context = QgsProcessingContext()
80+
res = processing.run('qgis:buffer',
81+
{'DISTANCE': 1, 'INPUT': points(), 'OUTPUT': QgsProcessing.TEMPORARY_OUTPUT},
82+
context=context, is_child_algorithm=True)
83+
self.assertIn('OUTPUT', res)
84+
# output should be a layer string reference, NOT the layer itself
85+
self.assertIsInstance(res['OUTPUT'], str)
86+
layer = context.temporaryLayerStore().mapLayer(res['OUTPUT'])
87+
self.assertIsInstance(layer, QgsVectorLayer)
88+
# context should have ownership
89+
self.assertFalse(sip.ispyowned(layer))
90+
del context
91+
gc.collect()
92+
self.assertTrue(sip.isdeleted(layer))
93+
94+
95+
if __name__ == '__main__':
96+
nose2.main()

‎python/plugins/processing/tools/general.py

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -89,11 +89,28 @@ def algorithmHelp(id):
8989
print('Algorithm "{}" not found.'.format(id))
9090

9191

92-
def run(algOrName, parameters, onFinish=None, feedback=None, context=None):
93-
"""Executes given algorithm and returns its outputs as dictionary
94-
object.
92+
def run(algOrName, parameters, onFinish=None, feedback=None, context=None, is_child_algorithm=False):
9593
"""
96-
return Processing.runAlgorithm(algOrName, parameters, onFinish, feedback, context)
94+
Executes given algorithm and returns its outputs as dictionary object.
95+
96+
:param algOrName: Either an instance of an algorithm, or an algorithm's ID
97+
:param parameters: Algorithm parameters dictionary
98+
:param onFinish: optional function to run after the algorithm has completed
99+
:param feedback: Processing feedback object
100+
:param context: Processing context object
101+
:param is_child_algorithm: Set to True if this algorithm is being run as part of a larger algorithm,
102+
i.e. it is a sub-part of an algorithm which calls other Processing algorithms.
103+
"""
104+
if onFinish or not is_child_algorithm:
105+
return Processing.runAlgorithm(algOrName, parameters, onFinish, feedback, context)
106+
else:
107+
# for child algorithms, we disable to default post-processing step where layer ownership
108+
# is transferred from the context to the caller. In this case, we NEED the ownership to remain
109+
# with the context, so that further steps in the algorithm have guaranteed access to the layer.
110+
def post_process(_alg, _context, _feedback):
111+
return
112+
113+
return Processing.runAlgorithm(algOrName, parameters, onFinish=post_process, feedback=feedback, context=context)
97114

98115

99116
def runAndLoadResults(algOrName, parameters, feedback=None, context=None):

0 commit comments

Comments
 (0)