<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Fri, Feb 2, 2018 at 12:36 AM, Nyall Dawson <span dir="ltr"><<a href="mailto:nyall.dawson@gmail.com" target="_blank">nyall.dawson@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="m_5684847473346299970m_-461032357978674772m_8542865208664759658m_-9202097866857776585m_-4212425059747350159gmail-">On 1 February 2018 at 20:01, Rashad Kanavath <<a href="mailto:mohammedrashadkm@gmail.com" target="_blank">mohammedrashadkm@gmail.com</a>> wrote:<br>
> Hello,<br>
><br>
> Processing plugin allows to integrate other toolboxes. IIUC, this was one of<br>
> the features of it.<br>
> So when you say integration of so and so toolboxes are total mess, you might<br>
> think back.<br>
<br>
</span>I think there's a misunderstanding/language barrier at play here -<br>
no-one meant that the code is a mess, just that the end result of the<br>
QGIS team trying to maintain 3rd party providers resulted in an<br>
inferior experience all round - for users, qgis developers AND the<br>
underlying library developers (whom I'm sure don't appreciate being<br>
blamed when a QGIS processing algorithm is mis-using their API).<br>
<br></blockquote><div> </div><div><span style="font-size:12.8px">Can you elaborate on this QGIS processing algorithm mis-using API ?</span></div><div><span style="font-size:12.8px"></span> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
Yes, one of Processing's big strengths is its ability to integrate<br>
other toolboxes and make 3rd party libraries accessible within the<br>
QGIS environment and tie them together into models. Another one of<br>
QGIS' great strengths is its strong plugin ecosystem, which allows<br>
anyone the ability to create plugins and extend QGIS, and not be tied<br>
into the core QGIS development practices and release schedules. That's<br>
why plugin-based providers are the BEST solution in my opinion.<br>
<span class="m_5684847473346299970m_-461032357978674772m_8542865208664759658m_-9202097866857776585m_-4212425059747350159gmail-"><br>
> Nobody had seen new changes to otb algs so all of your comments are on old<br>
> version.  Why so rush?<br>
> Okay, it easy to reject stating the same reason over and over again. I<br>
> understand that.<br>
<br>
</span>Just to be clear - no-one is commenting on your code quality or<br>
targeting OTB specifically. It's the question of whether or not<br>
Processing providers which depend on other libraries should be<br>
included in the main install or moved to plugins which we are<br>
debating. So please, don't take any of this as criticism of your code<br>
or (much appreciated) efforts.<br></blockquote><div><br></div><div style="font-size:12.8px">I never said you commented on code quality. I said everyone should comment on new code. </div><div style="font-size:12.8px">But there are comments and or decisions made on old one  codebase even the thread started with updating and cleaning up stuff.</div><div style="font-size:12.8px">And I did said that all your comments about that is valid and reasonable.</div><div style="font-size:12.8px"><br></div><div style="font-size:12.8px"><span style="font-size:12.8px">Hence, I suggested that to see if all your points to keep providers out still stands with new code. (not done yet)</span><br></div><div style="font-size:12.8px">For SAGA and others, I cannot speak much because I am not involved in it.  </div><div style="font-size:12.8px">so if it feels like I was targeting OTB, this was not against anything.</div><div style="font-size:12.8px"><span style="font-size:12.8px">But I guess saga or even grass is not planning for similar support for qgis provider like OTB.</span><br></div><div style="font-size:12.8px"><div style="font-size:12.8px"> And maybe this is the case for R and others. So that make sense to remove it from plugin.</div><div style="font-size:12.8px"><br></div><div style="font-size:12.8px">If there is one plugin provider which has mostly code tied to processing then it will be easy for QGIS. </div><div style="font-size:12.8px"><span style="font-size:12.8px">Consider this, OTB plugin was moved out of qgis some time ago and after that came a lot of changes to qgis processing.</span><br></div><div style="font-size:12.8px">change of parameter names, moving code to c++ etc.. Now GRASS, SAGA etc got all these changes because it was in tree.</div><div style="font-size:12.8px"><span style="font-size:12.8px">I understand the idea behind removing otb. If this provider wasn't such a mess to maintain it would have stayed. Even though it</span><br></div><div style="font-size:12.8px">attains the same complexity level as others. I don't agree that it should be so complex. </div><div style="font-size:12.8px"><br></div><div style="font-size:12.8px"><span style="font-size:12.8px">And IMHO, the point being this providers are hard to maintain itself lies with processing plugin and not individual providers or qgis core.</span><br></div><div style="font-size:12.8px">Be it OTB, GRASS , SAGA or anything. </div><div style="font-size:12.8px"><br></div></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<span class="m_5684847473346299970m_-461032357978674772m_8542865208664759658m_-9202097866857776585m_-4212425059747350159gmail-"><br>
> What happens at end, a processing plugin with zero providers?<br>
<br>
</span>Far from it. My vision would be:<br>
<br>
- core install: only includes the native QGIS providers (c++, Python<br>
and 3D) and the GDAL/OGR provider (since it's impossible to build QGIS<br>
without GDAL we can be 100% sure that it's always available wherever<br>
QGIS is installed). Maybe GRASS provider too, since GRASS is very<br>
heavily linked into QGIS due to the GRASS provider and c++ GRASS<br>
plugin.</blockquote><div style="font-size:12.8px">In case of GDAL, I agree. But I (and many other users) don't have GRASS and SAGA installed.</div><div style="font-size:12.8px">So this is some kind of assumption that I cannot fully agree. GRASS provider still has all of descriptor for grass7 in your tree.</div><div style="font-size:12.8px">And this was/is the case of OTB for now. New changes will not need a descriptor file in your tree.</div><div style="font-size:12.8px">OTB installation will provide a descriptor file in the format needed by qgis processing!</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> </blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
- via QGIS' plugin ecosystem: a whole world of providers ready to<br>
install for users, including SAGA, OTB, lastools, R, PySAL, etc...<br>
<span class="m_5684847473346299970m_-461032357978674772m_8542865208664759658m_-9202097866857776585m_-4212425059747350159gmail-"><br></span></blockquote><div><span style="font-size:12.8px"><br></span></div><div><span style="font-size:12.8px">Yes. most of the concern was more with installation of external tools for users.</span> </div><div><div style="font-size:12.8px">From this and other threads, keeping processing providers outside was not related to amount or quality code or targetted at specific tool.</div><div style="font-size:12.8px">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.</div><div style="font-size:12.8px">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.</div><div style="font-size:12.8px">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.</div></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="m_5684847473346299970m_-461032357978674772m_8542865208664759658m_-9202097866857776585m_-4212425059747350159gmail-">
> Now the reason OTB had to maintain list of "supported" version is due the<br>
> fact that processing plugin does not allow to group parameters.<br>
> So the issue of a provider being total mess not fully related to provider<br>
> itself. If 90% of provider algorithm which you use, cannot be even<br>
> integrated into processing where will be the actual problem. I see a lot of<br>
> good changes in qgis processing and providers can greatly benefit from it<br>
> now.<br>
<br>
</span>Can you elaborate on this please? I'm not aware what the "group<br>
parameters" issue is.<br></blockquote><div><span style="font-size:12.8px"><br></span></div><div><span style="font-size:12.8px">OTB application allows use of parameter with sub parameters,</span><br></div><div style="font-size:12.8px">One such example is here: </div><div style="font-size:12.8px"><a href="https://www.orfeo-toolbox.org/CookBook/Applications/app_Smoothing.html#parameters" target="_blank">https://www.orfeo-toolbox.org/<wbr>CookBook/Applications/app_Smoo<wbr>thing.html#parameters</a><br></div><div style="font-size:12.8px"><a href="https://www.orfeo-toolbox.org/CookBook/Applications/app_TrainImagesClassifier.html#parameters" target="_blank">https://www.orfeo-toolbox.org/<wbr>CookBook/Applications/app_Trai<wbr>nImagesClassifier.html#paramet<wbr>ers</a><br></div><div style="font-size:12.8px"><br></div><div style="font-size:12.8px">There is a parameter called "type" in Smoothing application. "type" parameter has list of values such as mean, gaussian and anidif. </div><div style="font-size:12.8px">The ui widget of this parameter is obviously a comboBox.</div><div style="font-size:12.8px">This flexibility allows users to select which type of smoothing to apply. Rather than having a three application namely</div><div style="font-size:12.8px">smoothing_mean, smoothing_gaussian, smoothing_anidif</div><div style="font-size:12.8px"><br></div><div style="font-size:12.8px">so parameter type.* (type.mean.radius, type.gaussian.radius, etc..) etc are child of parameter called "type". <br></div><div style="font-size:12.8px">So you can call "type" parameter a group. </div><div style="font-size:12.8px"><br></div><div style="font-size:12.8px">A user of application can select -type gaussian -type.gaussian.radius 3 when launching application.</div><div style="font-size:12.8px">otb application check for invalid cases such as if type is gaussian then one cannot use -type.mean.radius</div><div style="font-size:12.8px">This is okay for command line, but for graphical interface one has to hide /show parameters based on the value of it's parent.</div><div style="font-size:12.8px"><br></div><div style="font-size:12.8px">This is a limitation of qgis processing that upstream was forced to split applications based on group. I am not blaming or targeting here.</div><div style="font-size:12.8px">Just saying about a missing feature for support for one provider.</div><div style="font-size:12.8px"><br></div><div style="font-size:12.8px"> QGIS users now have three application for smoothing and maybe 10 or more application for TrainImagesClassifer. </div><div style="font-size:12.8px">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</div><div style="font-size:12.8px">And nobody wants to work on this part. </div><div style="font-size:12.8px"><br></div><div style="font-size:12.8px">Now, this can be fixed by allowing plugin providers to hide/show any of the parameters in the ui.</div><div style="font-size:12.8px">The rule to hide/show can be left out to provider itself.  This change must go in qgis processing and not in provider plugin.</div><div style="font-size:12.8px"><br></div><div style="font-size:12.8px">If qgis processing plugin has a new widget that allows this, then things go easy.  You can keep providers as external plugin.</div><div style="font-size:12.8px">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.</div><div style="font-size:12.8px">Provider will be free from hacks such as splitting applications and doing stuff specific to some version(s)</div><div style="font-size:12.8px">And this widget can be reused by existing or new providers also.<br></div><div style="font-size:12.8px"><br></div><div style="font-size:12.8px">I am attaching some screenshots from otb application viewer to visual explanation.</div><div style="font-size:12.8px">scheme_parameter.png shows the grouping of parameters.<br></div><div style="font-size:12.8px">and the other two explains the show/hide of parameters based on value of parameter "type"</div><div><span style="font-size:12.8px"><br></span></div><div><span style="font-size:12.8px">screenshots:</span></div><div><span style="font-size:12.8px"><a href="https://www.orfeo-toolbox.org/qgis/schema_parameter.png" target="_blank">https://www.orfeo-toolbox.org/<wbr>qgis/schema_parameter.png</a></span><br></div><div><span style="font-size:12.8px"><a href="https://www.orfeo-toolbox.org/qgis/Screenshot_smoothing_gaussian.png" target="_blank">https://www.orfeo-toolbox.org/<wbr>qgis/Screenshot_smoothing_gaus<wbr>sian.png</a></span><br></div><div><span style="font-size:12.8px"><a href="https://www.orfeo-toolbox.org/qgis/Screenshot_smoothing_anisotropic.png" target="_blank">https://www.orfeo-toolbox.org/<wbr>qgis/Screenshot_smoothing_anis<wbr>otropic.png</a></span><br></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<span class="m_5684847473346299970m_-461032357978674772m_8542865208664759658m_-9202097866857776585m_-4212425059747350159gmail-"><br>
> All those people blindly rejecting this proposal should have waited for new<br>
> changes.<br>
> I mean, I just put a proposal to lower the burden as much as possible. And<br>
> all those issues raised by Alex, Nyall and others will be considered in the<br>
> new diff.<br>
> Once I prepare changes, you can reject it!. You are voting -1ss on old<br>
> plugin code something I consider a mess to work with for both teams.<br>
<br>
</span>Why? In the past we've always found that discussing plans upfront<br>
benefits everyone and prevents development effort in an approach which<br>
won't be accepted upstream.<br>
<br>
In summary, can you please outline the reasons why you think<br>
publishing this as a plugin is not ideal?<br>
<span class="m_5684847473346299970m_-461032357978674772m_8542865208664759658m_-9202097866857776585m_-4212425059747350159gmail-HOEnZb"><font color="#888888"><br>
Nyall<br>
</font></span></blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="m_5684847473346299970m_-461032357978674772m_8542865208664759658m_-9202097866857776585m_-4212425059747350159gmail_signature"><div><font face="arial, helvetica, sans-serif">Regards,<br>   Rashad</font></div></div>
</div></div>