Add pattern fill for histogram, bar and barpolar traces #5520

s417-lama · 2021-02-23T14:16:35Z

Added pattern fill for bar plots ( #3815 ).

Although pattern fill could be applied to various types of plots, for now only bar plots are supported as a first step.

Supported attributes:

shape: borders (/, \, |, -), cross hatch (+, x), dots (.)
bgcolor: background color
scale: scaling factor of the preset shape of patterns
solidity: line width or radius of dots

Sample figure:

nicolaskruchten · 2021-02-23T15:07:13Z

Wow, awesome 😍 !

For reference, here is what Bokeh supports, including their little one-character abbreviations, for things we could add later: https://docs.bokeh.org/en/latest/docs/user_guide/styling.html#hatch-properties

src/components/drawing/index.js

src/traces/bar/attributes.js

alexcjohnson · 2021-02-24T00:14:05Z

@s417-lama very nice work! I've made a few comments but I think you're off to a great start, and this will be a popular feature!

src/components/drawing/index.js

src/components/legend/style.js

src/traces/bar/style_defaults.js

Co-authored-by: Alex Johnson <[email protected]>

alexcjohnson

Thanks for the updates @s417-lama - this looks ready to go from my side! 💃 @archmoj anything else from you?

The no-gl-jasmine test failure was one I had never seen before, it just couldn't find the test modules somehow? Anyhow it succeeded the second time.

archmoj · 2021-03-03T14:51:33Z

src/components/drawing/index.js

+                radius = Math.sqrt(solidity * size * size / Math.PI);
+            } else {
+                // linear interpolation
+                var linearFn = function(x, x0, x1, y0, y1) {


Could you declare this function outside the switch case please?

Fixed in 5161478

archmoj · 2021-03-03T14:56:51Z

src/components/drawing/index.js

            if(!gradientInfo[gradientType]) gradientType = 0;
        }

+        var getPatternAttr = function(mp, i, dflt) {


Non-blocking:
It looks like you could also move this function above drawing.pointStyle.

Fixed in 5161478

archmoj · 2021-03-03T14:58:07Z

src/components/drawing/index.js

+        var getPatternAttr = function(mp, i, dflt) {
+            if(mp && Array.isArray(mp)) {
+                if(i < mp.length) return mp[i];
+                else return dflt;


How about simply

return i < mp.length ? mp[i] : dflt;

Fixed in 5161478

archmoj · 2021-03-03T15:05:31Z

src/components/legend/style.js

+
+            var fillColor = d0.mc || marker.color;
+
+            var getPatternAttr = function(mp, dflt) {


This function looks quite similar to getPatternAttr in src/components/drawing/index.js.
Could you explain the difference between two?

getPatternAttr in legends does not require any index because the first element of the array is always used, but the same getPatternAttr function can be used for both. If we unify them, which location do you think is appropriate for putting the common getPatternAttr fn?

It could go in drawing.

Sure! Fixed in 5161478

archmoj · 2021-03-03T15:08:06Z

src/snapshot/tosvg.js

-                }
-            });
-        }
+        for(k in fullLayout._gradientUrlQueryParts) queryParts.push(k);


Can't we use slice instead of the for loop here and below?

In this regard, I couldn't see the point in storing query parts as a dict (_gradientUrlQueryParts) in the first place. Seems like only keys are used and values are not. Why don't we make it just an array?

Also, I suspect that it makes things more complicated to store query parts globally. How about just putting a dedicated class name for pattern fill (and gradient) and collecting them later? Actually, I already did the similar thing (below).

in components/drawing/index.js:

sel.classed('pattern_filled', true); var className2query = function(s) { return '.' + s.attr('class').replace(/\s/g, '.'); }; var k = className2query(d3.select(sel.node().parentNode)) + '>.pattern_filled'; fullLayout._patternUrlQueryParts[k] = 1;

I think it is enough to put .pattern_filled class and collect them by using a query for .pattern_filled when exporting svg. Maybe I am missing some points; is there any problem with it?

archmoj · 2021-03-03T15:11:56Z

src/traces/bar/style_defaults.js


    coerce('marker.line.width');
    coerce('marker.opacity');
+    var pattern = coerce('marker.pattern.shape');


Let's call this variable patternShape.

Fixed in 5161478

archmoj · 2021-03-03T15:17:47Z

src/traces/bar/style_defaults.js

+    if(pattern) {
+        coerce('marker.pattern.bgcolor');
+        coerce('marker.pattern.size');
+        coerce('marker.pattern.solidity');


We need to add a supplyDefaults test somewhere here for these attributes not be coerced when there is no pattern.shape.

archmoj · 2021-03-03T15:24:14Z

src/traces/bar/attributes.js

        editType: 'style',
        description: 'Sets the opacity of the bars.'
+    },
+    pattern: {


The bar/attributes is required and used by other traces namely barpolar, box, histogram, funnel, waterfall.
Are we planning to add pattern to any of those?

Technically it should be quite easy to apply pattern fill to other plots. I'm planning to support all of them and add some tests!

archmoj · 2021-03-03T15:26:22Z

src/components/drawing/index.js

+            var perPointPattern = Array.isArray(markerPattern.shape) ||
+                                  Array.isArray(markerPattern.bgcolor) ||
+                                  Array.isArray(markerPattern.size) ||
+                                  Array.isArray(markerPattern.solidity);


Shouldn't we use Lib.isArrayOrTypedArray here?

Fixed in 5161478

archmoj · 2021-03-03T15:28:20Z

src/components/legend/style.js

+            var fillColor = d0.mc || marker.color;
+
+            var getPatternAttr = function(mp, dflt) {
+                if(mp && Array.isArray(mp)) {


Should we handle typedArrays here as well?

Fixed in 5161478

archmoj · 2021-03-04T21:01:18Z

This PR also added pattern feature to histogram trace, which is cool!

Plotly.PlotSchema.get().traces.histogram.attributes.marker.pattern

So, let's add/expand bar image & jasmine tests to histogram and edit PR title & description.

archmoj · 2021-03-04T21:39:03Z

This PR also added pattern attribute to the schema of funnel trace marker.
To get that to work you need to coerce the new attributes in src/traces/funnel/defaults.js,
then add

Drawing.pointStyle(gTrace.selectAll('path'), trace, gd);

above this line

plotly.js/src/traces/funnel/style.js

Line 34 in ed52205

styleTextPoints(gTrace, trace, gd);

archmoj · 2021-03-04T21:44:32Z

This PR also added pattern feature to barpolar trace, which is cool!

Plotly.PlotSchema.get().traces.barpolar.attributes.marker.pattern

So, let's add/expand bar image & jasmine tests to barpolar as well and edit PR title & description.

s417-lama · 2021-03-08T05:02:29Z

I will handle the remaining issues (some tests and supports for plots other than bar plots) when I have time.

archmoj · 2021-03-08T14:34:01Z

I will handle the remaining issues (some tests and supports for plots other than bar plots) when I have time.

We would love to include this feature in the upcoming v2.0.0-rc.1.
@s417-lama wondering if you do not you have time to address those remanings today, I could merge this PR and help fix those?

s417-lama · 2021-03-08T14:44:57Z

@archmoj I don't think I have enough time today. Could you help fixing the remaining things? Thank you!

archmoj · 2021-03-08T14:53:12Z

@archmoj I don't think I have enough time today. Could you help fixing the remaining things? Thank you!

Sure. I'll take care of it.
Again thanks very much for the remarkable contribution! 🥇
💃

nicolaskruchten · 2021-03-08T15:00:26Z

Amazing contribution, thank you so much! 🙇

Add pattern fill for bar plots

8f53e11

archmoj added status: reviewable community community contribution feature something new labels Feb 23, 2021

fix lint warnings

863854f

alexcjohnson reviewed Feb 24, 2021

View reviewed changes

src/components/drawing/index.js Outdated Show resolved Hide resolved

alexcjohnson reviewed Feb 24, 2021

View reviewed changes

src/traces/bar/attributes.js Outdated Show resolved Hide resolved

alexcjohnson reviewed Feb 24, 2021

View reviewed changes

src/traces/bar/attributes.js Outdated Show resolved Hide resolved

alexcjohnson reviewed Feb 24, 2021

View reviewed changes

src/traces/bar/attributes.js Outdated Show resolved Hide resolved

alexcjohnson reviewed Feb 24, 2021

View reviewed changes

src/components/drawing/index.js Outdated Show resolved Hide resolved

Change the interface of pattern fill and some fix

39e35cc

alexcjohnson reviewed Mar 1, 2021

View reviewed changes

src/components/drawing/index.js Outdated Show resolved Hide resolved

alexcjohnson reviewed Mar 1, 2021

View reviewed changes

src/components/legend/style.js Outdated Show resolved Hide resolved

alexcjohnson reviewed Mar 1, 2021

View reviewed changes

src/traces/bar/style_defaults.js Outdated Show resolved Hide resolved

s417-lama and others added 2 commits March 3, 2021 15:14

Update src/traces/bar/style_defaults.js

9f7ce1d

Co-authored-by: Alex Johnson <[email protected]>

small fixes for pattern fill

6e9cb40

alexcjohnson approved these changes Mar 3, 2021

View reviewed changes

archmoj reviewed Mar 3, 2021

View reviewed changes

archmoj added this to the NEXT milestone Mar 4, 2021

small fixes

5161478

archmoj changed the title ~~Add pattern fill for bar plots~~ Add pattern fill for histogram, bar and barpolar traces Mar 8, 2021

archmoj merged commit c80fda4 into plotly:master Mar 8, 2021

archmoj mentioned this pull request Mar 8, 2021

Remove pattern from funnel attributes and add tests for histogram and barpolar #5537

Merged

alexcjohnson mentioned this pull request Mar 9, 2021

Revert "Add pattern fill for histogram, bar and barpolar traces" #5539

Closed

s417-lama mentioned this pull request Jan 30, 2022

Add pattern fill for scatter filled area #6101

Merged

thierryVergult mentioned this pull request May 12, 2023

pie : add Pattern #6601

Merged

Nick-Pearson mentioned this pull request Nov 15, 2024

Requesting patterns for bar charts, and histograms. plotly/plotly.rs#145

Closed


		var fillColor = d0.mc \|\| marker.color;

		var getPatternAttr = function(mp, dflt) {

Uh oh!

Add pattern fill for histogram, bar and barpolar traces #5520

Add pattern fill for histogram, bar and barpolar traces #5520

Uh oh!

Conversation

s417-lama commented Feb 23, 2021

Uh oh!

nicolaskruchten commented Feb 23, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alexcjohnson commented Feb 24, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alexcjohnson left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

archmoj commented Mar 4, 2021

Uh oh!

archmoj commented Mar 4, 2021

Uh oh!

archmoj commented Mar 4, 2021

Uh oh!

s417-lama commented Mar 8, 2021

Uh oh!

archmoj commented Mar 8, 2021

Uh oh!

s417-lama commented Mar 8, 2021

Uh oh!

archmoj commented Mar 8, 2021

Uh oh!

nicolaskruchten commented Mar 8, 2021

Uh oh!