search

GispoCoding / eis_qgis_plugin

A QGIS plugin for mineral prospectivity mapping
https://eis-he.eu/
8 stars 1 forks source link

Add surface derivatives to plugin #78

Closed lehtonenp closed 7 months ago

lehtonenp commented 8 months ago

This is a draft PR to view surface derivatives code style and implementation.

lehtonenp commented 8 months ago

Thank you for the feedback! I assumed that the first order, second order and classify aspect were required to be implemented as plugin functions. Now, I see that the higher order surface_derivatives_cli is implemented instead into the plugin. Did I understand correctly?

nmaarnio commented 8 months ago

Yes. This one is a bit unusual case as usually the toolkit functions match closely to the CLI implementation 😅. After some discussion with Micha, who implemented the surface derivative functions, we decided to simplify and streamline the way they are called in QGIS by combining first and second order functions into one (a bit more heavily parameterized) function.

There are some cases where a CLI functions execute two or more toolkit functions (for example data transformations before the "main" tool), and this is why the CLI function should always be looked at when implementing processing algorithms in our plugin.

nmaarnio commented 7 months ago

A quick comment here: The "description" field of each algorithm is actually the displayed name of the algorithm, and not a help string. It is unfortunately one of the misleading parameter names in QGIS. To add help strings to the algorithm parameters, we need to call setHelp method for the parameters (this has not been done so far in any implementation, but should be added eventually).

lehtonenp commented 7 months ago

A quick comment here: The "description" field of each algorithm is actually the displayed name of the algorithm, and not a help string. It is unfortunately one of the misleading parameter names in QGIS. To add help strings to the algorithm parameters, we need to call setHelp method for the parameters (this has not been done so far in any implementation, but should be added eventually).

I used gdal for reference in making the plugin work with the setHelp method. I could not call setHelp on initialization of QgsProcessingParameters.

nmaarnio commented 7 months ago

There was an issue with missing logic for enum parameters with multiple values. I added that and now this works, except that the produced layers are not automatically loaded since their number and namings are currently unknown to the algorithm definition. This is an issue that affects a few other algorithms too, and I think thinking it through needs a bit time and its better to handle them all at once. So I think this is in good enough state for merging.