EnMAP-Box / enmap-box

EnMAP-Box source code repository. See https://enmap-box.readthedocs.io for documentation
GNU General Public License v3.0
35 stars 16 forks source link

"Predict classification layer" can not use RFc model #504

Closed jakimowb closed 1 year ago

jakimowb commented 1 year ago

The "Predict classification layer" algorithm does not allow to use the classifier output of the "Fit RandomForestClassifier" output. Instead, it should be selectable as simple as the output raster from the "Import EnMAP L2A product"

grafik

janzandr commented 1 year ago

This is a general issue with custom widgets in processing algorithms: https://github.com/qgis/QGIS/issues/48816 QGIS DEVs decided to not fix it, because it uses deprecated API classes. Unfortunately, QGIS DEVs didn't told me which API classes to use instead and/or to provide an example implementation.

janzandr commented 1 year ago

I agree, that this would be an important issue to be solved.

janzandr commented 1 year ago

@jakimowb I prepared a minimal example reproducing the problem: image

https://github.com/EnMAP-Box/enmap-box/blob/504-predict-classification-layer-can-not-use-rfc-model/enmapboxprocessing/algorithm/ISSUE_504_ALGO.py

janzandr commented 1 year ago

So far I have no idea how to replace the deprecated WidgetWrapper class: image

janzandr commented 1 year ago

Note that even native QGIS algorithms still use the deprecated WidgetWrapper class. E.g. in the QGIS Raster Calculator the EXPRESSION parameter: image

Implementation is here: image

jakimowb commented 1 year ago

Have you checked the QGIS C++ code? This usually provides many example how the QGIS API can be used, in particular in case of those classes which were moved from pure python to C++ implementations

janzandr commented 1 year ago

My problem is, that there is no example implementation of a custom parameter widget, that isn't using the deprecated API.

janzandr commented 1 year ago

Have you checked the QGIS C++ code? This usually provides many example how the QGIS API can be used, in particular in case of those classes which were moved from pure python to C++ implementations

I tried to find something last time without success.

janzandr commented 1 year ago

So far I haven't found any custom widgets that aren't implemented via the deprecated class.

jakimowb commented 1 year ago

In that specific case it just requires a standard input parameter for the classifier *.pkl file. It is confusing that the custom widget can do all sorts of other things, but does not handle the basic case of accepting a previously created file.

janzandr commented 1 year ago

Maybe a crazy idea, but maybe good enought for now: I would suggest to have a second EnMAP-Box Algorithm Provider (e.g. "EnMAP-Box (no custom widgets)"), that is only visible in the modeller, which is providing a second version of the problematic algorithms without custom widgets. This would be rather quick to implement workaround, and we can at least start to build models.

I would guess that QGIS DEVs won't update their algorithms with custom widgets until the QGIS 4 release. At that point in time we just check how they did it and fix our algorithms accordingly.

janzandr commented 1 year ago

Ah, I just noticed that I already did that for the Fit RFC algo: image

Implementation is rather easy: image

janzandr commented 1 year ago

At the time I tried that, their was still a bug not hiding the other algo correctly in the modeller, but that issue seams to be solved in QGIS >3.28.

Now it looks very clean inside the GUI. The only detail a user might notice is the different algo ID, but that shouldn't really matter: image

janzandr commented 1 year ago

@jakimowb I would suggest to work around the issue by having the above descriped solution. Is that fine with you?

janzandr commented 1 year ago

The good thing is: we don't have to fix every algorithm individually, we just have to fix each custom input parameter. We just check at runtime, if the algorithm is used inside the modeller, if so, don't use the custom widget wrapper. E.g. image

janzandr commented 1 year ago

@jakimowb pointed out that there is an QGIS algo that isn't using the deprecated WidgetWrapper class for it's custom widget: image

I will check that out.

janzandr commented 1 year ago

I wasn't able to figure this out. But I have found an easy/hacky way to know if the algo is running inside the modeller, by just looking into the current tracestack. I just check if the algo is called by anything from the ModelerDialog.py: image

Now I can just decide at runtime, whether to use the custom widgets: image

This will fix the problems in any of our algorithms :-)