Skip to content

Conversation

@mechairoi
Copy link
Contributor

Added leastSquaresForecast linearRegression function for graphs the liner regression by least squares method.

localhost-2

@yuuki
Copy link
Contributor

yuuki commented Jan 8, 2016

👍

1 similar comment
@shoichimasuhara1982
Copy link

👍

@JeanFred
Copy link
Member

JeanFred commented Jan 8, 2016

Would you be able to provide a test for that new method?

@mechairoi
Copy link
Contributor Author

@JeanFred Thank you for the advice. I have written a test.

@Songmu
Copy link

Songmu commented Jan 13, 2016

@JeanFred Thank you for the review.

We heavily use graphite in our monitoring SaaS Mackerel for two years. @mechairoi he is in our team.

This feature is very useful for us (and all users, of cource).

We hope this pull request to be accepted.

@JeanFred
Copy link
Member

Thanks for writing this test @mechairoi!
This looks fine to me, but as a recent maintainer I would much prefer someone else has a look to it (poke @obfuscurity ?)

@obfuscurity
Copy link
Member

This is probably a dumb question, but does it make sense to default startSourceAt to the from parameter?

@mechairoi
Copy link
Contributor Author

These arguments are copied from the timeSlice function, but it sounds great.

Does it also make sense to default endSourceAt to the until as follows?

--- a/webapp/graphite/render/functions.py
+++ b/webapp/graphite/render/functions.py
@@ -2389,7 +2389,7 @@ def linearRegressionAnalysis(series):
     offset = (sumII * sumV - sumIV * sumI) /denominator - factor * series.start
     return factor, offset

-def linearRegression(requestContext, seriesList, startSourceAt, endSourceAt="now"):
+def linearRegression(requestContext, seriesList, startSourceAt=None, endSourceAt=None):
   """
   Graphs the liner regression function by least squares method.

@@ -2407,8 +2407,8 @@ def linearRegression(requestContext, seriesList, startSourceAt, endSourceAt="now
   """
   results = []
   sourceContext = requestContext.copy()
-  sourceContext['startTime'] = parseATTime(startSourceAt)
-  sourceContext['endTime'] = parseATTime(endSourceAt)
+  if startSourceAt is not None: sourceContext['startTime'] = parseATTime(startSourceAt)
+  if endSourceAt is not None: sourceContext['endTime'] = parseATTime(endSourceAt)

@mechairoi
Copy link
Contributor Author

@obfuscurity Thank you for the review. I fixed default startSourceAt and endSourceAt. Please let me know if you have any concerns.

@Songmu
Copy link

Songmu commented Feb 18, 2016

@obfuscurity @JeanFred
How about it?

@JeanFred
Copy link
Member

@Songmu Looks good to me ; but still too junior to press the merge button :)

@brutasse
Copy link
Member

@JeanFred do it :)

@JeanFred
Copy link
Member

All right, let’s do this then :) @mechairoi, would you be able to squash this?

@mechairoi mechairoi force-pushed the least-squares-forecast branch from 1af391c to 892c90f Compare February 29, 2016 15:39
@mechairoi
Copy link
Contributor Author

@JeanFred squashed :)

JeanFred added a commit that referenced this pull request Feb 29, 2016
Linear regression by least squares method
@JeanFred JeanFred merged commit 189ff9c into graphite-project:master Feb 29, 2016
@JeanFred
Copy link
Member

Aaaaaaand done!

@Songmu
Copy link

Songmu commented Feb 29, 2016

😄

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.

7 participants