resample() and smooth() functions

[obfuscurity]

This is a port of @danielbeardsley's work in #92 over to the master branch. Works fine for me although my attempt at adding a test for smooth() has been an utter failure. @cbowman0 you have time to look at this one?

[cbowman0]

I looked for a few minutes. The width parameter is causing everything to end with [None] bring returned. Reducing that to 10 started giving errors with resample. I'll look more later when I get time.

[cbowman0]

The range(20) call returns a list. No need for the braces around that.

[obfuscurity]

Good catch, that's an artifact from when I listed the members explicitly.

[cbowman0]

For anyone following along that is curious how I debug/write tests, I add the following code (or something similar) after the execution of the function to see what is being returned vs what was coded as expected:

        for index, series in enumerate(result):
          print series.getInfo()
          print expectedResults[index].getInfo()

This gets removed before checking in. Running

It may be worthwhile to make TimeSeries.__repr__ return more data than it currently does, so this is not necessary

[cbowman0]

@obfuscurity If the test was changed like this, it returns success, but I'm not 100% sure that it does as expected. The fact that certain inputs can result in no data being returned is concerning. Also, these inputs only hit the positive case on the if (len(series) < expectedSamples * 0.95): on line 1190 of functions.py

diff --git a/webapp/tests/test_functions.py b/webapp/tests/test_functions.py
index 5494b7d..84814a7 100644
--- a/webapp/tests/test_functions.py
+++ b/webapp/tests/test_functions.py
@@ -868,19 +868,19 @@ class FunctionsTest(TestCase):

     def test_smooth(self):
         seriesList = [
-            TimeSeries('collectd.test-db1.load.value',0,1,1,[range(20)]),
+            TimeSeries('collectd.test-db1.load.value',0,20,60,range(0, 1200, 60)),
         ]

         def mock_evaluateTokens(reqCtx, tokens, replacements=None):
             seriesList = [
-                TimeSeries('collectd.test-db1.load.value',0,1,1,[range(20)]),
+                TimeSeries('collectd.test-db1.load.value',0,100,12,range(0, 1200, 12)),
             ]
             for series in seriesList:
                 series.pathExpression = series.name
             return seriesList

         expectedResults = [
-            TimeSeries('smooth(collectd.test-db1.load.value, 5)',0,1,1,[None,None,None,None,2,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20])
+            TimeSeries('smooth(collectd.test-db1.load.value, 5)',60,100,12,range(0,1177,12)),
         ]

         with patch('graphite.render.functions.evaluateTokens', mock_evaluateTokens):
@@ -889,13 +889,16 @@ class FunctionsTest(TestCase):
                     'template': {},
                     'args': ({},{}),
                     'startTime': datetime(1970, 1, 1, 0, 0, 0, 0, pytz.timezone(settings.TIME_ZONE)),
-                    'endTime': datetime(1970, 1, 1, 0, 9, 0, 0, pytz.timezone(settings.TIME_ZONE)),
+                    'endTime': datetime(1970, 1, 1, 0, 20, 0, 0, pytz.timezone(settings.TIME_ZONE)),
                     'localOnly': False,
-                    'width': 400,
+                    'width': 100,
                     'data': []
                 },
                 seriesList, 5

[cbowman0]

Also, based on looking at the output of coverage (https://coverage.readthedocs.io) on my test vm via:

cbowman@cbo-virtualbox:~/git/graphite-web/webapp$ coverage html --include="graphite/*"
cbowman@cbo-virtualbox:~/git/graphite-web/webapp$ firefox htmlcov/graphite_render_functions_py.html 

those changes don't hit all of the resample code. So, only keeping that one test is insufficient. I can look more later tonight/tomorrow if need be.

[obfuscurity]

@cbowman0 You're a scholar and a gentleman. I'll wait for you to take a closer look later.

[deniszh]

Hello @obfuscurity,
Are you still interested in that functions? I can try to rebase, we can still put that to 1.0.0 :)

[obfuscurity]

@deniszh Yes but there's still a broken test.

[deniszh]

Ah, true. Maybe @cbowman0 will find some time to check this? I'm weak at tests. Or anyone else? /cc @graphite-project/committers ?

[cbowman0]

I will attempt to find time this week.

[deniszh]

Not sure do we still need this, but looks promising.
This is original description from #92 is

"Functions: add resample() for performance and sanity

Drastically speeds up further calcuations on the returned series
Makes it much easier to have a consistent datapoints / pixels ratio
for movingAverage() and friends.
Functions: add smooth() as a movingAverage over pixels

Internally does a movingAverage(resample()).
Provides consistent smoothing over a given number of graph pixels,
instead of a number of datapoints of arbitrary time width.
Still provides consistent smoothing when there are fewer datapoints
than pixels."

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.