[FEATURE] create_ramp() expression function #4632

nirvn · 2017-05-26T07:26:06Z

Description

This PR adds a useful expression function to create color ramps from scratch. It can be quite useful when a project in shared among many users as it does not rely on the presence of a saved ramp (let alone having that ramp having the same colors).

It's also simply useful to be able to quickly create ramps within expressions 😄

Checklist

Reviewing is a process done by project maintainers, mostly on a volunteer basis. We try to keep the overhead as small as possible and appreciate if you help us to do so by completing the following items. Feel free to ask in a comment if you have troubles with any of them.

Commit messages are descriptive and explain the rationale for changes
Commits which fix bugs include fixes #11111 in the commit message next to the description
Commits which add new features are tagged with [FEATURE] in the commit message
Commits which change the UI or existing user workflows are tagged with [needs-docs] in the commit message and containt sufficient information in the commit message to be documented
I have read the QGIS Coding Standards and this PR complies with them
This PR passes all existing unit tests (test results will be reported by travis-ci after opening this PR)
New unit tests have been added for core changes
I have run the scripts/prepare-commit.sh script before each commit

nirvn · 2017-05-26T07:26:28Z

@nyalldawson , review appreciated.

nirvn · 2017-05-26T07:40:36Z

Here's a practical example that can be achieved with this new function: graduate a polygon fill based on a numerical field against the field's maximum value: ramp_color(create_ramp(map(0,'255,255,255',1,'199,33,33')),scale_linear("my_field",0,maximum("my_field"),0,1))

nyalldawson · 2017-05-28T23:07:22Z

Looks good to me! My only thought - having a single function for both creation of the ramp AND retrieving a color from it is going to be inefficient for large number of calculations. If the functions were instead split into two separate functions then the ramp creation function would be considered static and compiled once (thanks for @m-kuhn's recent optimisations), meaning only one ramp creation instead of 1000s...

nirvn · 2017-05-29T00:21:21Z

@nyalldawson , yeah , I thought of that too, but initially I was not warm to the idea of introducing a new type of returned value in the expression engine (ie, a "ramp" value/object).

nyalldawson · 2017-05-29T00:41:43Z

@nyalldawson , yeah , I thought of that too, but initially I was not warm to the idea of introducing a new type of returned value in the expression engine (ie, a "ramp" value/object).

I understand that. However - there's also a good benefit of having ramps as a datatype. At the moment if you use the color property assistant and set a property to get its value from a custom ramp, there's no way to map this across to an expression function. We need some way to store custom ramps in expression functions to allow this. So this change would also benefit that use case...

nirvn · 2017-05-29T05:24:33Z

@nyalldawson , OK, following your request, I've updated this PR to allow for creation of ramps within the expression engine. The create_ramp() function creates a ramp, which can be used by the pre-existing ramp_color(). The latter was updated to take as its first parameter a ramp object in addition to a saved ramp name string.

The create_ramp() has two parameters: a required map() parameter, and an optional discrete boolean. This allows us to re-create all variants of the QgsGradientColorRamp ramp type. For e.g.:

create_ramp(map(0,'0,0,0',0.25,'255,0,0',1,'255,255,255'))
create_ramp(map(0,'0,0,0',0.5,'255,0,0'),true)
etc.

This practical example shows how a user can retrieve a color for a expression-created ramp:
ramp_color(create_ramp(map(0,'0,0,0',1,'255,0,0')),0.5)

The above would return '128,0,0,255'.

nyalldawson · 2017-05-29T06:03:08Z

src/core/expression/qgsexpressionfunction.cpp

-  const QgsColorRamp *mRamp = QgsStyle::defaultStyle()->colorRampRef( rampName );
-  if ( ! mRamp )
+  QgsGradientColorRamp expRamp;
+  const QgsColorRamp *mRamp;


this should be just ramp, not mRamp

nyalldawson · 2017-05-29T06:05:54Z

src/core/expression/qgsexpressionfunction.cpp

+    if ( ! mRamp )
+    {
+      parent->setEvalErrorString( QObject::tr( "\"%1\" is not a valid color ramp" ).arg( rampName ) );
+      return QColor( 0, 0, 0 ).name();


this is odd - i think it should return QVariant() instead

nyalldawson · 2017-05-29T06:07:18Z

tests/src/core/testqgsexpression.cpp

@@ -1010,6 +1010,8 @@ class TestQgsExpression: public QObject

      // Color functions
      QTest::newRow( "ramp color" ) << "ramp_color('Spectral',0.3)" << false << QVariant( "254,190,116,255" );
+      QTest::newRow( "create ramp color, two colors" ) << "ramp_color(create_ramp(map('0,0,0','255,0,0')),0.33)" << false << QVariant( "84,0,0,255" );
+      QTest::newRow( "create ramp color, three colors" ) << "ramp_color(create_ramp(map('0,0,0','0,255,0','0,0,255','255,0,0')),0.5)" << false << QVariant( "0,127,128,255" );


Need some tests here for bad parameters - e.g. 0 stops, 1 stop, non-map values

nyalldawson · 2017-05-29T06:08:02Z

Looks good! Just some minor suggestions.

Also - should we explicitly say that this is a gradient ramp creator, in case we want to allow random/etc ramp creation functions in future?

nirvn · 2017-05-29T06:13:22Z

@nyalldawson , as in updating the function name or adding a note in the documentation?

nyalldawson · 2017-05-29T06:16:00Z

I think just documentation

nirvn · 2017-05-29T07:05:18Z

@nyalldawson , your comments were all addressed. Thanks

DelazJ · 2017-05-29T07:22:06Z

resources/function_help/json/create_ramp

+{
+  "name": "create_ramp",
+  "type": "function",
+  "description": "Returns a gradient ramp from a an map of color strings and steps.",


Oups, used to be an array -- thanks for spotting this.

DelazJ · 2017-05-29T07:22:55Z

src/core/expression/qgsexpressionutils.h

+
+      // If we get here then we can't convert so we just error and return invalid.
+      if ( report_error )
+        parent->setEvalErrorString( QObject::tr( "Cannot convert '%1' to QgsGradientColorRamp" ).arg( value.toString() ) );


would the end user understand what QgsGradientColorRamp is?

@DelazJ , trying to stick to what was established elsewhere (see getGeometry(), getFeature()).

That should be fixed too - we shouldn't expose the class names to users. By the way - QgsExpression::formatPreviewString will need to be updated to show ramps too.

nirvn · 2017-05-29T08:43:27Z

@DelazJ , QgsGradientColorRamp renamed to "gradient ramp".

@nyalldawson , QgsExpression::formatPreviewString updated, thanks for pointing this out.

nirvn force-pushed the array_ramp_color branch from 1df1133 to f11dc11 Compare May 26, 2017 07:49

nirvn force-pushed the array_ramp_color branch from f11dc11 to bef8ed3 Compare May 29, 2017 05:20

nirvn changed the title ~~[FEATURE] array_ramp_color() expression function~~ [FEATURE] create_ramp() expression function May 29, 2017

nyalldawson reviewed May 29, 2017

View reviewed changes

nirvn force-pushed the array_ramp_color branch 2 times, most recently from 5e9d9ac to f7e78ee Compare May 29, 2017 07:04

DelazJ reviewed May 29, 2017

View reviewed changes

nirvn added 2 commits May 29, 2017 15:41

[FEATURE] create_ramp() expression function

551c25c

clean up expression convertion errors

4931f17

nirvn force-pushed the array_ramp_color branch from f7e78ee to 4931f17 Compare May 29, 2017 08:42

nirvn merged commit d996cf2 into qgis:master May 29, 2017

qgib mentioned this pull request May 24, 2019

Allow decimal degrees in Print composer #14699

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FEATURE] create_ramp() expression function #4632

[FEATURE] create_ramp() expression function #4632

nirvn commented May 26, 2017 •

edited

nirvn commented May 26, 2017

nirvn commented May 26, 2017 •

edited

nyalldawson commented May 28, 2017

nirvn commented May 29, 2017

nyalldawson commented May 29, 2017

nirvn commented May 29, 2017 •

edited

nyalldawson May 29, 2017

nyalldawson May 29, 2017

nyalldawson May 29, 2017

nyalldawson commented May 29, 2017

nirvn commented May 29, 2017

nyalldawson commented May 29, 2017

nirvn commented May 29, 2017

DelazJ May 29, 2017

nirvn May 29, 2017

DelazJ May 29, 2017

nirvn May 29, 2017

nyalldawson May 29, 2017

nirvn commented May 29, 2017

[FEATURE] create_ramp() expression function #4632

[FEATURE] create_ramp() expression function #4632

Conversation

nirvn commented May 26, 2017 • edited

Description

Checklist

nirvn commented May 26, 2017

nirvn commented May 26, 2017 • edited

nyalldawson commented May 28, 2017

nirvn commented May 29, 2017

nyalldawson commented May 29, 2017

nirvn commented May 29, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nyalldawson commented May 29, 2017

nirvn commented May 29, 2017

nyalldawson commented May 29, 2017

nirvn commented May 29, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nirvn commented May 29, 2017

nirvn commented May 26, 2017 •

edited

nirvn commented May 26, 2017 •

edited

nirvn commented May 29, 2017 •

edited