Code changes for Manhattan, Euclidean and Minkowski distance calculation #35
Conversation
…ation. * Plus reformatted code and added a common function to validate data.
|
@montanaflynn -Shivendra |
| @@ -0,0 +1,99 @@ | |||
| // | |||
There was a problem hiding this comment.
Can you remove these comments, they show up in the generated documentation:
https://godoc.org/github.com/montanaflynn/stats
The commit history and contribution is stored in the .git and GitHub.
data_set_distances.go
Outdated
| ) | ||
|
|
||
| // Validate data for distance calculation | ||
| func ValidateData(dataPointX, dataPointY []float64) error { |
There was a problem hiding this comment.
Let's make this a private function since it's only meant to be used internally. To do so just change the function name to validateData, Golang exposes uppercase functions, methods and variables.
data_set_distances_test.go
Outdated
| var err error | ||
| dataPointX := []float64{2, 3, 4, 5, 6, 7, 8} | ||
| dataPointY := []float64{8, 7, 6, 5, 4, 3, 2} | ||
| _, err = ChebyshevDistance(dataPointX, dataPointY) |
There was a problem hiding this comment.
Can we also check the result to be what is expected?
cd, err = ChebyshevDistance(dataPointX, dataPointY)
if err != nil {
t.Errorf("Failed to compute Chebyshev distance.")
}
if cd != 6 {
t.Errorf("ChebyshevDistance(%v, %v) => %.1f != %.1f", dataPointX, dataPointX, cd, 6)
}
data_set_distances_test.go
Outdated
| dataPointY := []float64{8, 7, 6, 5, 4, 3, 2} | ||
| _, err = ChebyshevDistance(dataPointX, dataPointY) | ||
| if err != nil { | ||
| t.Errorf("Failed to compute Chebyshev disatance.") |
data_set_distances_test.go
Outdated
|
|
||
| func TestDataSetDistances(t *testing.T) { | ||
| var err error | ||
| dataPointX := []float64{2, 3, 4, 5, 6, 7, 8} |
There was a problem hiding this comment.
Since I want to check the output as well it might be easier if you adopt the table driven test pattern:
examples/main.go
Outdated
| d, _ = stats.ChebyshevDistance([]float64{2, 3, 4, 5, 6, 7, 8}, []float64{8, 7, 6, 5, 4, 3, 2}) | ||
| fmt.Println(d) // Should yield 6 | ||
| d, _ = stats.ManhattanDistance([]float64{2, 3, 4, 5, 6, 7, 8}, []float64{8, 7, 6, 5, 4, 3, 2}) | ||
| fmt.Println(d) // Should yield 24 |
There was a problem hiding this comment.
Can we add some new lines between the functions to mimic the rest of the file?
d, _ = stats.ChebyshevDistance([]float64{2, 3, 4, 5, 6, 7, 8}, []float64{8, 7, 6, 5, 4, 3, 2})
fmt.Println(d) // Should yield 6
d, _ = stats.ManhattanDistance([]float64{2, 3, 4, 5, 6, 7, 8}, []float64{8, 7, 6, 5, 4, 3, 2})
fmt.Println(d) // Should yield 24
|
Hey @MishraShivendra, Sorry I was busy over the weekend. I added comments in a review. Thanks for your contributions! |
|
Thanks for those inputs. I'll push those changes in a while. -Shivendra |
|
@montanaflynn |
|
@MishraShivendra LGTM (looks good to me) and thanks for all your hard work and contributions! |
These changes would introduce functions to calculate Manhattan, Euclidean and Minkowski distance. Plus, I have done a bit of code reformatting in the old code.
Please take a look.
-Shivendra