Add reclassify raster to plugin

[lehtonenp]

This PR adds the following Reclassify raster functions to the plugin:

• Defined intervals
• Equal intervals
• Geometrical intervals
• Manual breaks
• Natural breaks
• Quantiles
• Standard deviation

The setHelp function is used to describe what the tools are for and Manual breaks contain an example input to the challenging breaks field.

[nmaarnio]

I quickly tested and Reclassify with defined intervals worked for me. So maybe the issue is in your toolkit environment somehow. Also, you should use QgsProcessingParameterBand for the raster band parameters.

[nmaarnio]

And add the minimum allowed values for the number parameters

[lehtonenp]

I quickly tested and Reclassify with defined intervals worked for me. So maybe the issue is in your toolkit environment somehow. Also, you should use QgsProcessingParameterBand for the raster band parameters.

Yes, definitely. I did not find that but I remembered that some tools in QGIS accept bands to be chosen like QgsProcessingParameterBand.

[lehtonenp]

And add the minimum allowed values for the number parameters

I have to admit that I don't know what the minValue's should be. Is it 0?

[nmaarnio]

I put some minimum values in the form of parameter checks to the toolkit functions in the fix PR I made. So just look at what the toolkit funcs accept currently.

[nmaarnio]

Hi, I did a quick review and you should define parentLayerParameterName parameter in all QgsProcessingParameterBands (self.alg_parameters[0]).

I would also personally remove the optional parameter from the QgsProcessingParameterBands even if it's actually optional in toolkit and not add allowMultiple on purpose. You can try it out how the UI looks with these on, but I think it can lead to confusion in some cases and I assume in most cases users have singleband rasters. Without optional, the parameter defaults to the first band which should always be present, and in case of multiband rasters maybe it's not too bad to process band-by-band if multiple bands need reclassification. However, feel free to disagree with this and implement with the optional and allowMultiple if you think they are useful.

[lehtonenp]

Hi, I did a quick review and you should define parentLayerParameterName parameter in all QgsProcessingParameterBands (self.alg_parameters[0]).

I would also personally remove the optional parameter from the QgsProcessingParameterBands even if it's actually optional in toolkit and not add allowMultiple on purpose. You can try it out how the UI looks with these on, but I think it can lead to confusion in some cases and I assume in most cases users have singleband rasters. Without optional, the parameter defaults to the first band which should always be present, and in case of multiband rasters maybe it's not too bad to process band-by-band if multiple bands need reclassification. However, feel free to disagree with this and implement with the optional and allowMultiple if you think they are useful.

I think all of your suggestions are worth implementing.

[nmaarnio]

Is this ready for review now?

[lehtonenp]

Is this ready for review now?

@nmaarnio, yes.

[lehtonenp]

2 algs don't run succesfully: Reclassify with quantiles (quantile param name doesn't match CLI) and Reclassify with manual breaks (no proper handling for the breaks given as string param). Please remember to test run the algorithms. Others work and seem ok.

Ok, thanks for the review. I am still unable to run the toolkit functions using the plugin.

I have also misunderstood the requirements for the plugin tool in manual breaks. I for some reason thought that breaks will be handled in the toolkit side. My apologies.

I wanted to inform that I got the plugin working successfully.

[lehtonenp]

no proper handling for the breaks given as string param

How should the handling be done? I have attempted various approaches but I am unable to figure out the correct way.

[nmaarnio]

How should the handling be done? I have attempted various approaches but I am unable to figure out the correct way.

I can try to implement the handling today, I'll let you know if I manage

[nmaarnio]

@lehtonenp I went for the easiest solution I could figure out, which is to override processAlgorithm method for this specific algorithm. But test if this works also on your machine to be sure. Is the PR now ready for another proper review if this implementation is ok?

[lehtonenp]

@lehtonenp I went for the easiest solution I could figure out, which is to override processAlgorithm method for this specific algorithm. But test if this works also on your machine to be sure. Is the PR now ready for another proper review if this implementation is ok?

@nmaarnio, this PR is ready for another review. Thank you for implementing the handling of breaks parameters. Your implementation worked.

[nmaarnio]

Did you fix the quantiles parameter yet? At least I don't see a commit involving that fix

[lehtonenp]

I had fixed it and I even committed the change. I just had not pushed the change. There should now be number_of_quantiles instead of quantiles as the parameter name.