Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[processing] Always list native algorithms before 3rd party providers #5369

Merged
merged 3 commits into from Oct 19, 2017

Conversation

nyalldawson
Copy link
Collaborator

In QGIS 3.0 we are trying to push users towards trying native QGIS algorithms first.

This change ensures that searching for algorithms always returns native algorithms before matching 3rd party algorithms:

image

Question: should we be removing the "[45 geoalgorithms]" text from the GDAL group? I don't see it as particular relevant and just additional noise, and it's also a bit odd in the above screenshot since there's clearly not 45 matching algorithms for that search phrase...

@nirvn
Copy link
Contributor

nirvn commented Oct 15, 2017

+1 to remove the number of algorirhms from 3rd party group labels.

I would go as far as using a shade of gray for matching 3rd party algs when searching.

@gioman
Copy link
Contributor

gioman commented Oct 15, 2017

+1 to remove the number of algorirhms from 3rd party group labels.

I would go as far as using a shade of gray for matching 3rd party algs when searching.

@nirvn @nyalldawson
I'm sorry, don't get me wrong, but why? I agree that the goal should be that, but:

  • GRASS, SAGA, OTB offer hundreds of tools that QGIS does not
  • Showing the number is a great way we have in trainings that QGIS offers anyway an amount of tools that is comparable to other geoprocessing toolboxes (arcgis anyone?)
  • many QGIS tools (i.e. the python ones) are not yet as robust (i.e. intersection, unions, etc) or fast (i.e dissolve) as they "3rd party" counterparts and in some case even returning wrong results or don't work at all with some inputs. Not I would not (and I don't) suggest them to be used, but they are so dangerous they should be removed until fixed.

@NathanW2
Copy link
Member

NathanW2 commented Oct 15, 2017 via email

@nyalldawson
Copy link
Collaborator Author

I would go as far as using a shade of gray for matching 3rd party algs when searching.

-1 on this... to me that conveys "disabled" or "inactive", neither of which applies here!

It's important to note that I'm not wanting to relegate external providers to second class citizens - instead I'm trying just to promote use of QGIS native algorithms as the suggested algorithms (for reasons discussed at length elsewhere).

@gioman

I'm sorry, don't get me wrong, but why?

To clarify, which parts are you against:

  1. the greying out of the algorithms (which as noted above is a -1 from me too)
  2. the change to sorting that this PR implements
  3. removal of the "45 geoalgorithm" count from the group name?

Re 3:

I HATE HATE HATE UI inconsistencies and complexity. In my opinion the algorithm count (and also the version number, but i'll discuss that later) violates both of these.

Check out this screenshot of master:

image

I'm sure we can agree that there's a consistency issue here - either every group needs the count, or none of them.

2.x approach is to show count for everything:

image

But I dislike that from a ui complexity viewpoint. There's way too much text here, and the actual useful bit ("GDAL"/"SAGA"/"GRASS"/"Models"/"Scripts" etc) is buried so far under all the extra text ("(2.3.2) [324 geoalgorithms]" / "GIS 7 commands [315 geoalgorithms]" etc) that it's a real strain to read this list and find what you're looking for. I'd bet most users rely on the icons alone for identifying groups.

My solutions would be (in order of preference):

  1. Drop the version number, count and all extraneous text ("...commands" / version numbers / suffixes like "commands" and "geoalgorithms")

  2. Drop the version number and extraneous text, change the count to be an unobtrusive "(45)" in a lighter color following the provider name (eg "GDAL (45)"). (Note that this is a real pain from a coding point of view - it'd need a custom delegate to handle the text coloring)

  3. Same as 2, but without the lighter count for algorithms

@alexbruy
Copy link
Contributor

+1 for algorithms count removal. This information is not necessary at all. Version number was useful some time ago when we supported multiple versions of same provider (e.g. SAGA 2.3.x and 2.2.x) but now it is also not really useful.

@nyalldawson
Copy link
Collaborator Author

I've pushed a commit which drops the extra text. Here's what it looks like with that in place:

image

Let me know if I should leave this commit in or not...

@nirvn
Copy link
Contributor

nirvn commented Oct 16, 2017

@nyalldawson , big +1 from me. I might have gone overboard with my suggestion to use gray after all 😉

@m-kuhn
Copy link
Member

m-kuhn commented Oct 16, 2017

Tooltips might also be a good possibility to show extra information (versions for 3rd party algs) in an unobtrusive way. What do you think @gioman ?

@gioman
Copy link
Contributor

gioman commented Oct 16, 2017

Tooltips might also be a good possibility to show extra information (versions for 3rd party algs) in an unobtrusive way. What do you think @gioman ?

sounds a good solution

This change ensures that searching for algorithms always returns
native algorithms before matching 3rd party algorithms

TODO: we really need to replace the toolbox tree with a proper
model and redo the sorting/filtering using a sort/filter proxy
model.
…ltips

Add version number to gdal provider long name
@nyalldawson
Copy link
Collaborator Author

sounds a good solution

Done!

@nyalldawson nyalldawson merged commit 8927fb5 into qgis:master Oct 19, 2017
@nyalldawson nyalldawson deleted the toolbox_order branch October 19, 2017 04:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants