improve auto ticks on axes with rangebreaks

plotly · archmoj · Jun 3, 2020 · Apr 6, 2020 · May 13, 2020 · May 13, 2020
commit a0d4d0d4d4bb336b3dcfd4c1dd58c24115eadf8d
diff --git a/src/plots/cartesian/axes.js b/src/plots/cartesian/axes.js
@@ -536,7 +536,7 @@ axes.prepTicks = function(ax) {
         if(ax.tickmode === 'array') nt *= 100;
 
 
-        ax._roughDTick = (Math.abs(rng[1] - rng[0]) - (ax._lBreaks || 0)) / nt;
+        ax._roughDTick = Math.abs(rng[1] - rng[0]) / nt;
         axes.autoTicks(ax, ax._roughDTick);
 
         // check for a forced minimum dtick
@@ -731,6 +731,25 @@ function arrayTicks(ax) {
     return ticksOut;
 }
 
+function roundBaseDay(dayHours) {
+    switch(dayHours) {
+        case 4: return [1, 2];
+        case 6: return [1, 2, 3];
+        case 8: return [1, 2, 4];
+        case 9: return [1, 3];
+        case 10: return [1, 2, 5];
+        case 12: return [1, 2, 3, 4, 6];
+        case 14: return [1, 2, 7];
+        case 15: return [1, 3, 5];
+        case 16: return [1, 2, 4, 8];
+        case 18: return [1, 2, 3, 6, 9];
+        case 20: return [1, 2, 4, 5, 10];
+        case 21: return [1, 3, 7];
+        case 22: return [1, 2, 11];
+    }
+    return [1];
+}
+
 var roundBase10 = [2, 5, 10];
 var roundBase24 = [1, 2, 3, 6, 12];
 var roundBase60 = [1, 2, 5, 10, 15, 30];
@@ -776,22 +795,34 @@ axes.autoTicks = function(ax, roughDTick) {
         // being > half of the final unit - so precalculate twice the rough val
         var roughX2 = 2 * roughDTick;
 
+        var oneDay = ONEDAY;
+        var dayRatio = 1;
+        if(ax._hasHourBreaks) {
+            oneDay = ax._dayHours * ONEHOUR;
+            dayRatio = Math.round(ax._dayHours / 24 * 7) / 7; // we use this in week context
+        }
+
         if(roughX2 > ONEAVGYEAR) {
             roughDTick /= ONEAVGYEAR;
             base = getBase(10);
             ax.dtick = 'M' + (12 * roundDTick(roughDTick, base, roundBase10));
         } else if(roughX2 > ONEAVGMONTH) {
             roughDTick /= ONEAVGMONTH;
             ax.dtick = 'M' + roundDTick(roughDTick, 1, roundBase24);
-        } else if(roughX2 > ONEDAY) {
-            ax.dtick = roundDTick(roughDTick, ONEDAY, ax._hasDayOfWeekBreaks ? [1, 7, 14] : roundDays);
-
+        } else if(roughX2 > oneDay) {
+            ax.dtick = roundDTick(roughDTick, oneDay, ax._hasDayOfWeekBreaks ?
+                [1, 2 * dayRatio, 7 * dayRatio, 14 * dayRatio] :
+                roundDays
+            );
             // get week ticks on sunday
             // this will also move the base tick off 2000-01-01 if dtick is
             // 2 or 3 days... but that's a weird enough case that we'll ignore it.
             ax.tick0 = Lib.dateTick0(ax.calendar, true);
         } else if(roughX2 > ONEHOUR) {
-            ax.dtick = roundDTick(roughDTick, ONEHOUR, roundBase24);
+            ax.dtick = roundDTick(roughDTick, ONEHOUR, ax._hasHourBreaks ?
+                roundBaseDay(ax._dayHours) :
+                roundBase24
+            );
         } else if(roughX2 > ONEMIN) {
             ax.dtick = roundDTick(roughDTick, ONEMIN, roundBase60);
         } else if(roughX2 > ONESEC) {

diff --git a/src/plots/cartesian/axis_defaults.js b/src/plots/cartesian/axis_defaults.js
@@ -135,11 +135,22 @@ module.exports = function handleAxisDefaults(containerIn, containerOut, coerce,
         if(!containerOut.rangebreaks.length) {
             delete containerOut.rangebreaks;
         } else {
+            var n = 0;
             for(var k = 0; k < containerOut.rangebreaks.length; k++) {
-                if(containerOut.rangebreaks[k].pattern === DAY_OF_WEEK) {
+                var brk = containerOut.rangebreaks[k];
+                if(brk.pattern === DAY_OF_WEEK) {
                     containerOut._hasDayOfWeekBreaks = true;
-                    break;
+                    n++;
                 }
+
+                if(brk.pattern === HOUR) {
+                    containerOut._hasHourBreaks = true;
+                    containerOut._dayHours = Math.round((brk.bounds[1] - brk.bounds[0] + 24) % 24);
+                    n++;
+                }
+
+                // break when found all types
+                if(n === 2) break;
             }
 
             setConvert(containerOut, layoutOut);

diff --git a/test/image/baselines/axes_breaks-candlestick2.png b/test/image/baselines/axes_breaks-candlestick2.png
diff --git a/test/image/baselines/axes_breaks-contour1d.png b/test/image/baselines/axes_breaks-contour1d.png
diff --git a/test/image/baselines/axes_breaks-contour2d.png b/test/image/baselines/axes_breaks-contour2d.png
diff --git a/test/image/baselines/axes_breaks-dtick_auto.png b/test/image/baselines/axes_breaks-dtick_auto.png
diff --git a/test/image/baselines/axes_breaks-finance.png b/test/image/baselines/axes_breaks-finance.png
diff --git a/test/image/baselines/axes_breaks-heatmap1d.png b/test/image/baselines/axes_breaks-heatmap1d.png
diff --git a/test/image/baselines/axes_breaks-heatmap2d.png b/test/image/baselines/axes_breaks-heatmap2d.png
diff --git a/test/image/baselines/axes_breaks-histogram2d.png b/test/image/baselines/axes_breaks-histogram2d.png
diff --git a/test/image/baselines/axes_breaks-night_autorange-reversed.png b/test/image/baselines/axes_breaks-night_autorange-reversed.png
diff --git a/test/image/baselines/axes_breaks-rangeslider.png b/test/image/baselines/axes_breaks-rangeslider.png
diff --git a/test/image/baselines/axes_breaks-reversed-without-pattern.png b/test/image/baselines/axes_breaks-reversed-without-pattern.png
diff --git a/test/image/baselines/axes_breaks-round-weekdays.png b/test/image/baselines/axes_breaks-round-weekdays.png
diff --git a/test/image/baselines/axes_breaks-values.png b/test/image/baselines/axes_breaks-values.png
diff --git a/test/image/baselines/axes_breaks-weekends-weeknights.png b/test/image/baselines/axes_breaks-weekends-weeknights.png
diff --git a/test/image/baselines/axes_breaks-weekends_autorange-reversed.png b/test/image/baselines/axes_breaks-weekends_autorange-reversed.png
diff --git a/test/image/baselines/axes_breaks.png b/test/image/baselines/axes_breaks.png
diff --git a/test/jasmine/tests/axes_test.js b/test/jasmine/tests/axes_test.js
@@ -4977,7 +4977,7 @@ describe('Test axes', function() {
                     .withContext(msg).toEqual(exp.tickVals);
             }
 
-            it('should include requested ticks that fall within rangebreaks', function(done) {
+            it('should not include requested ticks that fall within rangebreaks', function(done) {
                 Plotly.plot(gd, [{
                     x: [
                         '1970-01-01 00:00:00.000',
@@ -5016,7 +5016,7 @@ describe('Test axes', function() {
                 })
                 .then(function() {
                     _assert('with two rangebreaks', {
-                        tickVals: [0, 5, 10, 90, 95, 100, 190, 195, 200]
+                        tickVals: [0, 89, 100, 200]
                     });
                 })
                 .catch(failTest)
-Original file line number
+Diff line change
@@ Expand Up @@
             if(!containerOut.rangebreaks.length) {
                 delete containerOut.rangebreaks;
             } else {
+                var n = 0;
                 for(var k = 0; k < containerOut.rangebreaks.length; k++) {
-                    if(containerOut.rangebreaks[k].pattern === DAY_OF_WEEK) {
+                    var brk = containerOut.rangebreaks[k];
+                    if(brk.pattern === DAY_OF_WEEK) {
                         containerOut._hasDayOfWeekBreaks = true;
-                        break;
+                        n++;
                     }
+                    if(brk.pattern === HOUR) {
+                        containerOut._hasHourBreaks = true;
+                        containerOut._dayHours = Math.round((brk.bounds[1] - brk.bounds[0] + 24) % 24);
+                        n++;
+                    }
+                    // break when found all types
+                    if(n === 2) break;
             Copy link

  
      
    
  

  
      

  
    Collaborator


      

  

  
    
      

      
            alexcjohnson
  

      

      

      


        May 27, 2020


      
    

  


        
      
  
  
  
    

    There was a problem hiding this comment.


  

 
  
    

    Choose a reason for hiding this comment

    
      The reason will be displayed to describe this comment to others. Learn more.
    

    
      
      


  


  
    
      what if the user includes two breaks of one type, THEN a break of the other type? I suppose I can theoretically imagine wanting two disjoint hour periods - a morning shift and an evening shift, with a midday break? I wouldn't worry about doing something "pretty" in this case, perhaps just set _dayHours to something like 1 or 2 so we go straight from day to hour ticks or perhaps just use the first break. But it definitely shouldn't stop us from noticing _hasDayOfWeekBreaks
And vice versa I guess... maybe someone wants to skip Sundays and Wednesdays? Again, weird, but shouldn't stop us from catching _hasHourBreaks
    
  
  


    

        
      
  
  
    
  
  
  
    
    Sorry, something went wrong.
  

  
    
  
    
      

              Uh oh!

              
There was an error while loading. Please reload this page.


  
  


          
      
  
    
    
      
        
            
    All reactions
                 }
                 setConvert(containerOut, layoutOut);
@@ Expand Down @@