[PROCESSING] Speed up dissolve (esp. when using dissolve field)

[bstroebl]

No description provided.

[pcav]

@volaya @alexbruy

[m-kuhn]

It's dangerous to request the index from the layer and then use it to get a field from the provider.
I would try to avoid direct access to the provider wherever possible. field = vlayerA.field(fieldname)should be sufficient.

[bstroebl]

Hi Matthias,
thanks for the hint. Will recode.

[m-kuhn]

@bstroebl I tested it on a small file here and the dissolve field (string) was not respected.

It would be great to have a unit test come along with it (it's not a requirement but I'll buy you a beer next time if you sponsor the first ftools unit test)

[bstroebl]

Matthias, did you untick Dissolve all (do not use field)? If yes, could you send me the data, please?
Concerning the UI I would prefer a different behaviour. I assume most people use a field to dissolve (otherwise they could just use merge selected features from the Advanced Digitizing toolbar), so the preferred behaviour would be to

1. either have Dissolve all unticked by default but then there should be a way to make sure a field has been chosen before clicking Run
2. or have the checkbox unticked when a field is chosen

[m-kuhn]

Tried again and could not reproduce the error.

Can you clarify on untick dissolve all? For me there is no checkbox, it's just another entry in the combobox. Am I testing the wrong tool now?

[bstroebl]

This is Dissolve from QGIS geoalgorithms, UI displayed below

[m-kuhn]

That's the same one from the Vector > Geoprocessing Tools menu.

It seems to me that in the processing dialog the checkbox is superfluous?

[bstroebl]

Probably you are right, so if unique id field is [not set] then dissolve all would be assumed. The derivation of the field index could be put into a try-except block to check if a field has been chosen. The labels would need to clearly state this behaviour.

[m-kuhn]

Sounds good. Or adding logic to disable the combobox when the checkbox is checked.

[bstroebl]

Would be the better solution but I do not know if in Processing you can change widgets' settings when user sets other widgets. Would need Qt's signal/slot stuff.

[m-kuhn]

No idea, I never did a processing GUI. If it's not possible, your proposal sounds absolutely fine to me.

[bstroebl]

So these two would work:

1. Keeping the checkbox but
   * unticking it by default and
   * having the field as compulsory parameter (will be ignored if checkbox is ticked)
2. Keeping only the field combo but
   * still as an optional parameter and
   * dissolve all if [not set] is chosen
   What do you think is better understandable?

[m-kuhn]

I think it is confusing to have two different GUI controls which may be set to contradicting states - or having enabled widgets which do not have any meaning.
And if it's not possible to disable the combobox based on the checkbox that would be the case, right?
Therefore I would opt for #2.

[bstroebl]

ok, I will implement No 2 then
Pull request will be updated soon