Add logical operators to processing algos

[chrstnbwnkl]

First, thanks for this amazingly helpful plugin!

This PR adds logical operators as parameters to the processing algorithms. The issue was that using multiple keys/values did not work properly in the processing algorithms, so they could only be used for simple queries with one k/v pair each. I have added a string parameter that one can add comma separated 'AND'/'OR's to that will be used to combine multiple keys and values into one query.

I have further added one test, and one existing test needed a small adjustment, because there was a misplaced positional argument that now conflicted with this new feature.

[Gustry]

Nice addition, the multi key was indeed missing from the Processing toolbox.
I was thinking to provide a UI widget to make it easier. Because, in the Processing framework, it's possible to provide a custom UI widget within the plugin to make a kind of custom input widget.
The "Refactor fields" algorithm in QGIS native is one example, the table is not provided by QGIS native widgets but from the algorithm itself.

Have you already tried this other approach ?

[nilsnolde]

Hi @Gustry im working with @chrstnbwnkl , just my 2c:)

I considered that multi k/v UI before in another plugin, I think I just couldn’t make it work easily and quickly gave up😅 would be a much better approach.

What do you think about having this as an initial implementation and move to the nicer version in a follow-up PR? The reason I’m asking is we’d like the functionality in a release soonish, so we can use it for a project. We can commit to the follow-up PR as well actually in Feb.

[nilsnolde]

quick update @Gustry : we have no more deadline on this, so we can take our time. if you want we'll implement it including a friendlier UI as you suggested?

[Gustry]

Hi, sorry for the long delay.

Yes, I was expecting your answer ;-)

What do you think about having this as an initial implementation and move to the nicer version in a follow-up PR?

The thing is as soon as we publish something in the QGIS Processing algorithm for INPUTS or OUTPUTS, we can't change it until the next QuickOSM API break (that I would like to happen as late as possible), because Procesing is a kind of API for the plugin. It's used by custom Python scripts and by Processing models.

Removing or updating INPUTS/OUTPUTS will make a pain for model creator. When QGIS core developers are doing similar things in QGIS core Processing algorithms, they need to hide legacy parameters to keep existing models/python scripts etc working.

Let me try your PR first ;-)

[nilsnolde]

We'll have a look how much pain it is to stay backwards-compatible (if possible, would need to have a deeper look). if not, we'll just publish the PR branch on our internal qgis repo and call it smth else so clients don't get confused;)

[Gustry]

A quick look in your PR, it seems really not difficult to keep it backward compatible if the table is added.
I will take some times to try your PR ASAP ;-)

[Gustry]

Hi, sorry for the late review.

I just fixed the lint and pushed to your branch.

The feature seems OK. With these input widgets, difficult to make something very user-friendly. But it's ok ;-)

Can you expand the help on the parameter and on the algorithm ? I guess most users won't be sure how to use this new field.
Can you add a test in the test_query_factory ? (without the Processing part). I can have a look if needed.

Thanks

[Gustry]

AFAIK, except in the name key, I don't see too many other keys in which the value can contain a comma.
If a user write name=something,something, this would be an error if I'm correct in your PR ?

I mean there are keys description, note ... but the probability is low to search a comma in the value... isn't it ?

[chrstnbwnkl]

Sorry for letting this go stale again for so long... I checked tags of an .osm file I had lying around and you're right indeed: tags where values include a comma are almost exclusively description, note and similar tags that contain full text. I would say it's rather unlikely that users search for such specific tag values. If they do nonetheless, we can place a hint in the error message that is shown when the number of keys/values/(logical operators - 1) does not match up. What do you think?

[Bavarello]

I don't know if this request will have any possibility of development... but as far as I'm concerned I found this limitation to be of no small importance.

In any case, thanks for this incredibly useful plugin.

[Gustry]

@chrstnbwnkl I just rebased your PR, fix the conflict and pushed the branch again.

CI failing tests are not related to this PR it seems. I will have a look on the master branch first.

[chrstnbwnkl]

What a nice surprise 😄 still there remains the question of how to act on users including commas in their query strings IIRC

[Gustry]

Very sorry for the late review ... I forgot about it :/

It's summer, I want to release a new version... Thanks for your contribution !