More processing parameters in modeller #6504
Conversation
to manage parameter metadata in a single place.
|
Wow! Super nice addition |
|
Nice. BTW, side thought, I think ultimately we want the parameters to be added through a dedicated toolbar instead of a tree list widget. That'd make the experience feel a lot more like our layout designer. |
|
@alexbruy could you give this a review? |
I like the treeview with filter possibilities, but I think they should be in the same treeview as the algorithms. |
|
Ready for review |
|
Looks good to me. Only wondering if we should have parameters registry in the core instead of having it in Python. |
|
+1 to have parameters in the treeview with filter. With custom parameters we very quickly will get cluttered toolbar. |
|
Would there be any use for a basic parameters toolbar to live alongside the treeview. |
It's kind of a mixed thing between widgets (which are defined in python) and core parameters (which are in C++). And wasn't sure how to expose classes (the param['parameter'] thing) from C++ side. So I'm not unhappy with this as a first solution. If the requirements come up or someone can think of a clean solution, it should be easy to do that down the road.
I'd do it like QtCreator. Add elements from the treeview and have a toolbar for layout things and other operations. |
|
Yep, I also think that as first solution it looks awesome. We can improve things later when needed |
|
Late comment (very small merge window here!) But why not add a flag for parameters as to whether they are exposed to the modeler and filter on this? Then we have a method to hide parameters which CANNOT be used as model parameters. |
| Processing.REGISTERED_PARAMETERS = dict() | ||
|
|
||
| @staticmethod | ||
| def registerParameter(id, name, parameter, metadata=dict(), description=None, exposeToModeller=True): |
There was a problem hiding this comment.
Can we move this to the C++ processing registry please, and add unit tests for it?
That's in there already.
See #6504 (comment) |
|
And sorry for the quick merge, happy for any review to still land now ! |
Ahh - I was thinking of adding a new flag to QgsProcessingParameterDefinition.Flag, but I see now that that won't work since you'd need an instance of the definition to check the flags. My concern with adding this in Python is that it becomes part of the stable API - so it's locked in for the next 3-4 months (4.0 is June-ish, right?*). (We could potentially add some bridge from this API to a future c++ core parameter registry)
I think the c++ solution would be very similar to how renderers, symbol layers, layout items, etc are registered. You'd create a metadata object with virtual methods for creating the parameter and creating a config widget (with an enum specifying whether it's for toolbox/modeler/batch). ** a total lie |
|
By the way -- don't get me wrong, this is a great feature and much appreciated! |
Right now we don't promis much API stability within processing, or that shouldn't happen (and I think it should happen). I guess at some point we'll need to start tagging methods and attributes in processing as API stable.
Yeah, it's the "MetaParameter" object that is missing. And which would result in writing a QgsProcessingParameterRegistry and QgsProcessingParameterFactories, pretty much like what the editor widgets (and all the others you mentioned...) And I only wanted to have the fieldsmapper available... |
|
Ok, how about:
How's that sound? |
|
Unit tests for methods that are not stable API sounds not like what I want to spend my time on to be honest... I totally understand the point. But if I do something from now on, I'd much rather implement a real solution than spending more time on the current implementation (which b.t.w. I think is an improvement already). |
Not trying to be a pain here, because you know I love your work and I understand the reluctance in spending more time on this, but that argument doesn't really hold up. A regression is a regression regardless of whether the underlying code is in stable api or not! 😉
Let me know if you do want to go down this route... I've got ideas on how we can slowly start to transition processing gui base classes to c++. I think I've even got an ancient branch floating around somewhere with some first steps... |
https://github.com/qgis/QGIS/compare/master...m-kuhn:procparams?expand=1 |
|
It's beautiful 😭 |
Opens up the processing parameter system more. Instead of having hardcoded lists of parameters in various places, there is a registry of available parameters and an API for plugins to expose their own parameters.
This makes the fields mapping parameter for refactor fields available in the modeller.
As a nice side effect, the list of available parameters in the modeller dialog is now also translatable.
At the current state, all available parameters are exposed to the modeller. Many of them are not prepared (i.e. they don't have a combobox to select from available input parameter definitions inside an algorithm, so cannot be connected). I'll probably disable those for now, they can then incrementally be added later on.