Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[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}
  • Loading branch information
nyalldawson committed Jan 29, 2019
1 parent 82ec141 commit 7f7c7a9
Show file tree
Hide file tree
Showing 3 changed files with 118 additions and 4 deletions.
1 change: 1 addition & 0 deletions python/plugins/processing/tests/CMakeLists.txt
Expand Up @@ -6,6 +6,7 @@ PLUGIN_INSTALL(processing tests/testdata ${TEST_DATA_FILES})

IF(ENABLE_TESTS)
INCLUDE(UsePythonTest)
ADD_PYTHON_TEST(ProcessingGeneralTest ProcessingGeneralTest.py)
ADD_PYTHON_TEST(ProcessingGuiTest GuiTest.py)
ADD_PYTHON_TEST(ProcessingModelerTest ModelerTest.py)
ADD_PYTHON_TEST(ProcessingProjectProviderTest ProjectProvider.py)
Expand Down
96 changes: 96 additions & 0 deletions python/plugins/processing/tests/ProcessingGeneralTest.py
@@ -0,0 +1,96 @@
# -*- coding: utf-8 -*-

"""
***************************************************************************
QgisAlgorithmTests.py
---------------------
Date : January 2019
Copyright : (C) 2019 by Nyall Dawson
Email : nyall dot dawson at gmail dot com
***************************************************************************
* *
* This program is free software; you can redistribute it and/or modify *
* it under the terms of the GNU General Public License as published by *
* the Free Software Foundation; either version 2 of the License, or *
* (at your option) any later version. *
* *
***************************************************************************
"""

__author__ = 'Nyall Dawson'
__date__ = 'January 2019'
__copyright__ = '(C) 2019, Nyall Dawson'

# This will get replaced with a git SHA1 when you do a git archive

__revision__ = ':%H$'

import nose2
import shutil
import gc

from qgis.core import (QgsApplication,
QgsProcessing,
QgsProcessingContext,
QgsVectorLayer)
from qgis.PyQt import sip
from qgis.analysis import (QgsNativeAlgorithms)
from qgis.testing import start_app, unittest
import processing
from processing.tests.TestData import points


class TestProcessingGeneral(unittest.TestCase):

@classmethod
def setUpClass(cls):
start_app()
from processing.core.Processing import Processing
Processing.initialize()
QgsApplication.processingRegistry().addProvider(QgsNativeAlgorithms())
cls.cleanup_paths = []
cls.in_place_layers = {}
cls.vector_layer_params = {}

@classmethod
def tearDownClass(cls):
from processing.core.Processing import Processing
Processing.deinitialize()
for path in cls.cleanup_paths:
shutil.rmtree(path)

def testRun(self):
context = QgsProcessingContext()

# try running an alg using processing.run - ownership of result layer should be transferred back to the caller
res = processing.run('qgis:buffer',
{'DISTANCE': 1, 'INPUT': points(), 'OUTPUT': QgsProcessing.TEMPORARY_OUTPUT},
context=context)
self.assertIn('OUTPUT', res)
# output should be the layer instance itself
self.assertIsInstance(res['OUTPUT'], QgsVectorLayer)
# Python should have ownership
self.assertTrue(sip.ispyowned(res['OUTPUT']))
del context
gc.collect()
self.assertFalse(sip.isdeleted(res['OUTPUT']))

# now try using processing.run with is_child_algorithm = True. Ownership should remain with the context
context = QgsProcessingContext()
res = processing.run('qgis:buffer',
{'DISTANCE': 1, 'INPUT': points(), 'OUTPUT': QgsProcessing.TEMPORARY_OUTPUT},
context=context, is_child_algorithm=True)
self.assertIn('OUTPUT', res)
# output should be a layer string reference, NOT the layer itself
self.assertIsInstance(res['OUTPUT'], str)
layer = context.temporaryLayerStore().mapLayer(res['OUTPUT'])
self.assertIsInstance(layer, QgsVectorLayer)
# context should have ownership
self.assertFalse(sip.ispyowned(layer))
del context
gc.collect()
self.assertTrue(sip.isdeleted(layer))


if __name__ == '__main__':
nose2.main()
25 changes: 21 additions & 4 deletions python/plugins/processing/tools/general.py
Expand Up @@ -89,11 +89,28 @@ def algorithmHelp(id):
print('Algorithm "{}" not found.'.format(id))


def run(algOrName, parameters, onFinish=None, feedback=None, context=None):
"""Executes given algorithm and returns its outputs as dictionary
object.
def run(algOrName, parameters, onFinish=None, feedback=None, context=None, is_child_algorithm=False):
"""
return Processing.runAlgorithm(algOrName, parameters, onFinish, feedback, context)
Executes given algorithm and returns its outputs as dictionary object.
:param algOrName: Either an instance of an algorithm, or an algorithm's ID
:param parameters: Algorithm parameters dictionary
:param onFinish: optional function to run after the algorithm has completed
:param feedback: Processing feedback object
:param context: Processing context object
:param is_child_algorithm: Set to True if this algorithm is being run as part of a larger algorithm,
i.e. it is a sub-part of an algorithm which calls other Processing algorithms.
"""
if onFinish or not is_child_algorithm:
return Processing.runAlgorithm(algOrName, parameters, onFinish, feedback, context)
else:
# for child algorithms, we disable to default post-processing step where layer ownership
# is transferred from the context to the caller. In this case, we NEED the ownership to remain
# with the context, so that further steps in the algorithm have guaranteed access to the layer.
def post_process(_alg, _context, _feedback):
return

return Processing.runAlgorithm(algOrName, parameters, onFinish=post_process, feedback=feedback, context=context)


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

0 comments on commit 7f7c7a9

Please sign in to comment.