[QGIS-Developer] Keeping OTB algorithm in qgis processing

Fri Feb 2 02:47:29 PST 2018

On Fri, Feb 2, 2018 at 12:36 AM, Nyall Dawson <nyall.dawson at gmail.com>
wrote:

> On 1 February 2018 at 20:01, Rashad Kanavath <mohammedrashadkm at gmail.com>
> wrote:
> > Hello,
> >
> > Processing plugin allows to integrate other toolboxes. IIUC, this was
> one of
> > the features of it.
> > So when you say integration of so and so toolboxes are total mess, you
> might
> > think back.
>
> I think there's a misunderstanding/language barrier at play here -
> no-one meant that the code is a mess, just that the end result of the
> QGIS team trying to maintain 3rd party providers resulted in an
> inferior experience all round - for users, qgis developers AND the
> underlying library developers (whom I'm sure don't appreciate being
> blamed when a QGIS processing algorithm is mis-using their API).
>
>
Can you elaborate on this QGIS processing algorithm mis-using API ?

> Yes, one of Processing's big strengths is its ability to integrate
> other toolboxes and make 3rd party libraries accessible within the
> QGIS environment and tie them together into models. Another one of
> QGIS' great strengths is its strong plugin ecosystem, which allows
> anyone the ability to create plugins and extend QGIS, and not be tied
> into the core QGIS development practices and release schedules. That's
> why plugin-based providers are the BEST solution in my opinion.
>
> > Nobody had seen new changes to otb algs so all of your comments are on
> old
> > version.  Why so rush?
> > Okay, it easy to reject stating the same reason over and over again. I
> > understand that.
>
> Just to be clear - no-one is commenting on your code quality or
> targeting OTB specifically. It's the question of whether or not
> Processing providers which depend on other libraries should be
> included in the main install or moved to plugins which we are
> debating. So please, don't take any of this as criticism of your code
> or (much appreciated) efforts.
>

I never said you commented on code quality. I said everyone should comment
on new code.
But there are comments and or decisions made on old one  codebase even the
thread started with updating and cleaning up stuff.
And I did said that all your comments about that is valid and reasonable.

Hence, I suggested that to see if all your points to keep providers out
still stands with new code. (not done yet)
For SAGA and others, I cannot speak much because I am not involved in it.
so if it feels like I was targeting OTB, this was not against anything.
But I guess saga or even grass is not planning for similar support for qgis
provider like OTB.
 And maybe this is the case for R and others. So that make sense to remove
it from plugin.

If there is one plugin provider which has mostly code tied to processing
then it will be easy for QGIS.
Consider this, OTB plugin was moved out of qgis some time ago and after
that came a lot of changes to qgis processing.
change of parameter names, moving code to c++ etc.. Now GRASS, SAGA etc got
all these changes because it was in tree.
I understand the idea behind removing otb. If this provider wasn't such a
mess to maintain it would have stayed. Even though it
attains the same complexity level as others. I don't agree that it should
be so complex.

And IMHO, the point being this providers are hard to maintain itself lies
with processing plugin and not individual providers or qgis core.
Be it OTB, GRASS , SAGA or anything.

> > What happens at end, a processing plugin with zero providers?
>
> Far from it. My vision would be:
>
> - core install: only includes the native QGIS providers (c++, Python
> and 3D) and the GDAL/OGR provider (since it's impossible to build QGIS
> without GDAL we can be 100% sure that it's always available wherever
> QGIS is installed). Maybe GRASS provider too, since GRASS is very
> heavily linked into QGIS due to the GRASS provider and c++ GRASS
> plugin.

In case of GDAL, I agree. But I (and many other users) don't have GRASS and
SAGA installed.
So this is some kind of assumption that I cannot fully agree. GRASS
provider still has all of descriptor for grass7 in your tree.
And this was/is the case of OTB for now. New changes will not need a
descriptor file in your tree.
OTB installation will provide a descriptor file in the format needed by
qgis processing!

>

- via QGIS' plugin ecosystem: a whole world of providers ready to
> install for users, including SAGA, OTB, lastools, R, PySAL, etc...
>
>
Yes. most of the concern was more with installation of external tools for
users.
>From this and other threads, keeping processing providers outside was not
related to amount or quality code or targetted at specific tool.
Just that it requires external tools. We understand that totally. But what
we don't agree is your assumption that so and so tools is already for most
of qgis users.
Even if grass or saga is installed, one has to configure it to use. So the
problem of installing something for users seems to be key here.
Now GDAL is not considered an external tool because you need it to build
qgis. I am talking of other providers and not just OTB.

> > Now the reason OTB had to maintain list of "supported" version is due the
> > fact that processing plugin does not allow to group parameters.
> > So the issue of a provider being total mess not fully related to provider
> > itself. If 90% of provider algorithm which you use, cannot be even
> > integrated into processing where will be the actual problem. I see a lot
> of
> > good changes in qgis processing and providers can greatly benefit from it
> > now.
>
> Can you elaborate on this please? I'm not aware what the "group
> parameters" issue is.
>

OTB application allows use of parameter with sub parameters,
One such example is here:
https://www.orfeo-toolbox.org/CookBook/Applications/app_Smoo
thing.html#parameters
https://www.orfeo-toolbox.org/CookBook/Applications/app_Trai
nImagesClassifier.html#parameters

There is a parameter called "type" in Smoothing application. "type"
parameter has list of values such as mean, gaussian and anidif.
The ui widget of this parameter is obviously a comboBox.
This flexibility allows users to select which type of smoothing to apply.
Rather than having a three application namely
smoothing_mean, smoothing_gaussian, smoothing_anidif

so parameter type.* (type.mean.radius, type.gaussian.radius, etc..) etc are
child of parameter called "type".
So you can call "type" parameter a group.

A user of application can select -type gaussian -type.gaussian.radius 3
when launching application.
otb application check for invalid cases such as if type is gaussian then
one cannot use -type.mean.radius
This is okay for command line, but for graphical interface one has to hide
/show parameters based on the value of it's parent.

This is a limitation of qgis processing that upstream was forced to split
applications based on group. I am not blaming or targeting here.
Just saying about a missing feature for support for one provider.

 QGIS users now have three application for smoothing and maybe 10 or more
application for TrainImagesClassifer.
This make users of  otb provider as well as developers of both qgis and otb
unhappy.  Perimeter of bug report is getting more and more
And nobody wants to work on this part.

Now, this can be fixed by allowing plugin providers to hide/show any of the
parameters in the ui.
The rule to hide/show can be left out to provider itself.  This change must
go in qgis processing and not in provider plugin.

If qgis processing plugin has a new widget that allows this, then things go
easy.  You can keep providers as external plugin.
But provider will make use a qgs processing ui widget which is already in
tree.  That makes sure it is tested properly among other things.
Provider will be free from hacks such as splitting applications and doing
stuff specific to some version(s)
And this widget can be reused by existing or new providers also.

I am attaching some screenshots from otb application viewer to visual
explanation.
scheme_parameter.png shows the grouping of parameters.
and the other two explains the show/hide of parameters based on value of
parameter "type"

screenshots:
https://www.orfeo-toolbox.org/qgis/schema_parameter.png
https://www.orfeo-toolbox.org/qgis/Screenshot_smoothing_gaussian.png
https://www.orfeo-toolbox.org/qgis/Screenshot_smoothing_anisotropic.png

> > All those people blindly rejecting this proposal should have waited for
> new
> > changes.
> > I mean, I just put a proposal to lower the burden as much as possible.
> And
> > all those issues raised by Alex, Nyall and others will be considered in
> the
> > new diff.
> > Once I prepare changes, you can reject it!. You are voting -1ss on old
> > plugin code something I consider a mess to work with for both teams.
>
> Why? In the past we've always found that discussing plans upfront
> benefits everyone and prevents development effort in an approach which
> won't be accepted upstream.
>
> In summary, can you please outline the reasons why you think
> publishing this as a plugin is not ideal?
>
> Nyall
>

-- 
Regards,
   Rashad
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osgeo.org/pipermail/qgis-developer/attachments/20180202/465b0f3b/attachment-0001.html>