JuliaImages / ImageDistances.jl

Distances between N-dimensional images
https://github.com/JuliaImages/Images.jl
Other
15 stars 8 forks source link

WIP: minkowski distance and related mae, mse, psnr, etc #6

Closed johnnychen94 closed 5 years ago

johnnychen94 commented 5 years ago

Not sure if it's proper to create a psnr PR here

codecov[bot] commented 5 years ago

Codecov Report

Merging #6 into master will decrease coverage by 40.9%. The diff coverage is 0%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master       #6       +/-   ##
===========================================
- Coverage     100%   59.09%   -40.91%     
===========================================
  Files           3        4        +1     
  Lines          52       88       +36     
===========================================
  Hits           52       52               
- Misses          0       36       +36
Impacted Files Coverage Δ
src/minkowski.jl 0% <0%> (ø)
src/hausdorff.jl 66.66% <0%> (-33.34%) :arrow_down:
src/generic.jl 68.18% <0%> (-31.82%) :arrow_down:
src/ciede2000.jl 75% <0%> (-25%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update f0a303f...1a8a680. Read the comment docs.

juliohm commented 5 years ago

Thanks for the contribution! Can you also double check that the definitions you are adding here include the ones in Images.jl? https://github.com/JuliaImages/Images.jl/blob/master/src/algorithms.jl#L274-L384

It would be nice to have all the distances defined here, and then eliminate that code in Images.jl in a future PR. I will review the code with more time later. Got sick this week, currently at home resting.

Best,

johnnychen94 commented 5 years ago

It would be nice to have all the distances defined here, and then eliminate that code in Images.jl in a future PR.

Should I keep the same APIs? For example, the ssd(sum of squared differences) and sad(sum of absolute differences) there are just minkowski_phere

juliohm commented 5 years ago

I think minkowski is enough. Maybe we add alias later after the PR is ready.

On Sat, Nov 3, 2018, 01:15 Johnny Chen <notifications@github.com wrote:

It would be nice to have all the distances defined here, and then eliminate that code in Images.jl in a future PR.

Should I keep the same APIs? For example, the ssd(sum of squared differences) and sad(sum of absolute differences) there are just minkowski_phere

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/JuliaImages/ImageDistances.jl/pull/6#issuecomment-435559198, or mute the thread https://github.com/notifications/unsubscribe-auth/ADMLbb4fChr6-FujX3wqiyNAqCWbTAqtks5urRh6gaJpZM4YLk25 .

johnnychen94 commented 5 years ago

I didn't notice the existence of Distances package. Some codes here seem to be duplicated and unnecessary -- Both have Minkowski defined. The test fails because of this conflict.

@juliohm Not very clear what should I do next, should I close this PR or continue it? Pros of this PR is that it supports types such as N0fx, Gray and RGB.

juliohm commented 5 years ago

That is a good point. Distances.jl already has Minkowski. Does it for images out of the box? If not, we could provide the methods and conversions to make it work.

On Sun, Nov 4, 2018, 06:48 Johnny Chen <notifications@github.com wrote:

I didn't notice the existence of Distances package. Some codes here seem to be duplicated and unnecessary -- Both have Minkowski defined.

@juliohm https://github.com/juliohm Not very clear what should I do next, should I close this PR or continue it?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/JuliaImages/ImageDistances.jl/pull/6#issuecomment-435652246, or mute the thread https://github.com/notifications/unsubscribe-auth/ADMLbcjVbVwSSJO6qN2c_8iVmPwVRHHuks5urqnLgaJpZM4YLk25 .

johnnychen94 commented 5 years ago

Close this PR since I've accidentally deleted the branch :(

I'll make a PR to adapt the new design https://github.com/JuliaImages/ImageDistances.jl/issues/7 first, then work on this.