Github user iyerr3 commented on the pull request: https://github.com/apache/incubator-madlib/pull/42#issuecomment-218301178 General comments: - The distance functions (mean_*_error) all have the same structure except the distance metric. I suggest refactoring the table creation and the grouping statements into a single function that takes a parameter for the distance. - Avoid using `__` within python. It leads to [name mangling](https://docs.python.org/2/tutorial/classes.html#private-variables-and-class-local-references), that doesn't really help in this situation. - There seems to be considerable overlap between the grouping and non-grouping SQL in `r2`, `adjusted_r2` and `auc` functions. Maybe we can combine them? - I'll bump the [suggestion](https://github.com/apache/incubator-madlib/pull/41#issuecomment-216661096) by @decibel in the previous PR. It might be helpful to have 2 versions: 1 that creates the output table and the other returns the rows directly.
--- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---