bstroebl / DigitizingTools

A QGIS plugin, that subsumes different tools useful during digitizing sessions
GNU General Public License v2.0
22 stars 9 forks source link

Enable "Merge Selected Features" for all data providers #39

Closed andywicht closed 5 years ago

andywicht commented 5 years ago

Hi Bernhard,

The "Merge Selected Features" is inactive when editing Shapefiles. I suspect that it is connected to the logic of automatically merging a feature which does not have an ID (PK field) yet, which you recently implemented. As there is no defined PK field in the shapefile, the tool is inactive. Would it be possible to make it aware of the provider and always bring up the dialogue when merging features in a Shapefile?

QGIS: 3.2.1 DigitizingTools: 1.1.5

bstroebl commented 5 years ago

Hi Andy, this is on purpose (see the tool's documentation in the wiki). If you are editing shapefiles you can use QGIS' out-of-the-box merge features tool.

andywicht commented 5 years ago

Hi Bernhard, I see. The way your version of Merge selected Features work is so much more convenient than the core one, especially when dealing with a large number of merges in a row. So it would be great to also work on local files!

bstroebl commented 5 years ago

I do not see that much difference. It is selecting the feature to stay by its primary key (DigitizingTools) versus selecting the feature to stay and click Take attributes from selected feature in the Merge Feature Attributes dialogue (Core). Besides: how would you identify the feature to stay if there was no primary key?

andywicht commented 5 years ago

It is selecting the feature to stay by its primary key (DigitizingTools) versus selecting the feature to stay and click Take attributes from selected feature in the Merge Feature Attributes dialogue (Core)

The dropdown menu you implemented in your version is a lot more convenient for a user and reduces the room for error (the click on Take attributes from selected feature is omitted).

Besides: how would you identify the feature to stay if there was no primary key?

That's why I thought it would be possible to always bring up the dialogue to choose the feature (using the internal feature id? is that possible?) and not offer the "shortcut" when merging a freshly split off part with an existing feature (as implemented at the moment in DigitizingTools).

bstroebl commented 5 years ago

The dropdown menu you implemented in your version is a lot more convenient for a user and reduces the room for error (the click on Take attributes from selected feature is omitted).

Agreed: there is one click less but on the other hand there is no choice to save attribute values from different features. It is all about the balance between choice and ease of use. DigitizingTools solves a problem in a certain scenario: The layer is a database layer with a primary key and its key field is a foreign key in other tables. When using the core tool all features are deleted and all datasets in related tables are deleted, too, if the foreign key has ON DELETE CASCDE or the merge is prohibited if the foreign key has ON DELETE RESTRICT.

That's why I thought it would be possible to always bring up the dialogue to choose the feature (using the internal feature id? is that possible?) and not offer the "shortcut" when merging a freshly split off part with an existing feature (as implemented at the moment in DigitizingTools).

Wait a minute, I am confused, I do not understand what you want to tell me. The dialogue is only brought up if a primary key exists; this thread is about editing shapefiles which do not have a primary key, so there is no dialogue because the tool is not active in the first place. The internal id is of course possible, but it does have no meaning for the user, does it? If you merge five features how would you decide which is which?

andywicht commented 5 years ago

DigitizingTools solves a problem in a certain scenario: The layer is a database layer with a primary key and its key field is a foreign key in other tables. When using the core tool all features are deleted and all datasets in related tables are deleted, too, if the foreign key has ON DELETE CASCDE or the merge is prohibited if the foreign key has ON DELETE RESTRICT.

And it does that very elegantly, that is why I also see a use for it outside of a PostGIS context.

Wait a minute, I am confused, I do not understand what you want to tell me. The dialogue is only brought up if a primary key exists; this thread is about editing shapefiles which do not have a primary key, so there is no dialogue because the tool is not active in the first place.

Yes, it would have to be made aware of the provider and handle them differently.

The internal id is of course possible, but it does have no meaning for the user, does it? If you merge five features how would you decide which is which?

Often a primary key is equally meaningless on its own (at least that is my experience). I see that the feature which stays during a merging process (e.g. of 5 or more features) is picked visually. I do that using the Down Arrow key to quickly skip through the dropdown menu (dropdown menu has to be opened and closed before that to be possible; I do that by clicking twice on the first entry), waiting for the right feature to be highlighted on the canvas -> hit Enter and see the result.

bstroebl commented 5 years ago

Ahh, now I understand! I was not aware of the highlighting any more because I do not use the tools that often myself :-) I agree that the primary key is not meaningful, either, but contrary to the internal id the user can at least easily check it out with the i-Button or in the table view.

andywicht commented 5 years ago

I hear you... The curse of a developer ;-)

The internal Feature ID is shown in the "Derived" section of the Identify Results pane, isn't it? Or is that again a different internal ID?

bstroebl commented 5 years ago

I normally directly open the form because I hate the Identify Results panel

bstroebl commented 5 years ago

Hi Andy I implemented this in the develop branch. Could you check it out and try, please? Feedback welcome.

andywicht commented 5 years ago

When I try to start QGIS I get the following error:

Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/qgis/utils.py", line 337, in startPlugin
    plugins[packageName] = package.classFactory(iface)
  File "/home/aw/.local/share/QGIS/QGIS3/profiles/default/python/plugins/DigitizingTools/__init__.py", line 28, in classFactory
    from .digitizingtools import DigitizingTools
  File "/usr/lib/python3/dist-packages/qgis/utils.py", line 674, in _import
    mod = _builtin_import(name, globals, locals, fromlist, level)
  File "/home/aw/.local/share/QGIS/QGIS3/profiles/default/python/plugins/DigitizingTools/digitizingtools.py", line 36, in 
    import dtsplitmultipart
  File "/usr/lib/python3/dist-packages/qgis/utils.py", line 674, in _import
    mod = _builtin_import(name, globals, locals, fromlist, level)
  File "/home/aw/.local/share/QGIS/QGIS3/profiles/default/python/plugins/DigitizingTools/tools/dtsplitmultipart.py", line 27, in 
    import dt_icons_rc
  File "/usr/lib/python3/dist-packages/qgis/utils.py", line 674, in _import
    mod = _builtin_import(name, globals, locals, fromlist, level)
ModuleNotFoundError: No module named 'dt_icons_rc'
bstroebl commented 5 years ago

Ah. you need to install Make and call it or it is probably easier for you to simply replace dtmerge.py in tools in your intalled plugin.

andywicht commented 5 years ago

Perfect! Works as expected. The only thing I would change is to rename the string "id" in the dropdown menu to something more fitting reflecting that it is the internal feature id ("fid"? "int. fid"? "qgis fid"?). The column name "id" is common in attribute tables and might lead to confusion.

bstroebl commented 5 years ago

I am going to use Feature ID as in the Identify Results panel.

andywicht commented 5 years ago

Yup, awesome!

BTW I am still getting the module not found error after running make in the plugin root. dt_icons_rc.py is successfully created in the root folder of the plugin.

bstroebl commented 5 years ago

Maybe dt_icons_rc.py is not copied? I am glad it runs here as I am a lay on make

andywicht commented 5 years ago

Hi Bernhard, I just tried it on my windows box and the QGIS throws following error message when merging: 2018-08-08T09:18:35 CRITICAL DigitizingTools : The geometry type of the result is not valid in this layer! Same layer, same polygons work just fine when using linux.

Both machines have DigitizingTools 1.1.5 with exchanged dtmerge.py

QGIS Windows

QGIS version  3.2.1-Bonn
QGIS code revision  1edf372fb8
Compiled against Qt  5.9.2
Running against Qt  5.9.2
Compiled against GDAL/OGR  2.2.4
Running against GDAL/OGR  2.2.4
Compiled against GEOS  3.6.1-CAPI-1.10.1
Running against GEOS  3.6.1-CAPI-1.10.1 r0
PostgreSQL Client Version  9.2.4
SpatiaLite Version  4.3.0
QWT Version  6.1.3
QScintilla2 Version  2.10.1
PROJ.4 Version 493

QGIS Linux

QGIS version | 3.2.1-Bonn |
QGIS code revision | 1edf372
Compiled against Qt | 5.9.5 |
Running against Qt | 5.9.5
Compiled against GDAL/OGR | 2.2.3 | 
Running against GDAL/OGR | 2.2.3
Compiled against GEOS | 3.6.2-CAPI-1.10.2 |
Running against GEOS | 3.6.2-CAPI-1.10.2 4d2925d6
PostgreSQL Client Version | 10.3 (Ubuntu 10.3-1) |
SpatiaLite Version | 4.3.0a
QWT Version | 6.1.3 |
QScintilla2 Version | 2.10.2
PROJ.4 Version | 493
bstroebl commented 5 years ago

Hi Andy, I tried to fix this issue in the develop branch. Could you please download tools/dttools.py and have a try? I cannot test this here.

andywicht commented 5 years ago

Hi Bernhard, that seems to have fixed it, didn't do a super thorough test though. Thank you!