-
Notifications
You must be signed in to change notification settings - Fork 27
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
swirl operation #146
base: master
Are you sure you want to change the base?
swirl operation #146
Conversation
Codecov Report
@@ Coverage Diff @@
## master #146 +/- ##
===========================================
- Coverage 89.06% 70.44% -18.63%
===========================================
Files 8 9 +1
Lines 192 159 -33
===========================================
- Hits 171 112 -59
- Misses 21 47 +26
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you also help update the demo with some info comment such as
In this example, we illustrate how to construct a custom warping map and pass it to warp. This swirl example comes from the Princeton Computer Graphics course for Image Warping (Fall 2000) and scikit-image swirl example.
+ To apply the swirl effect, please use the existing `swirl` function in ImageTransformations.jl directly.
How to write tests for things like this?
This is a thin wrapper on the low-level interface warp
, so as long as we ensure the arguments are computed and passed correctly, it should be fine. A similar example is imrotate
test/swirl.jl
Outdated
replace!(res, NaN=>0.0) | ||
replace!(expected, NaN=>0.0) | ||
@test isapprox(res, expected; atol=0.1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's nearlysame
ImageTransformations.jl/test/runtests.jl
Lines 10 to 11 in 4d89e5c
nearlysame(x, y) = x ≈ y || (isnan(x) & isnan(y)) | |
nearlysame(A::AbstractArray, B::AbstractArray) = all(map(nearlysame, A, B)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue arose mainly from here
julia> expected[2,2]
0.431797
julia> res[2,2]
0.4317972472093078
julia> res[2,2] ≈ expected[2,2]
false
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nearlysame(x, y, atol = 0.0) = isapprox(x, y; atol = atol) || (isnan(x) & isnan(y))
nearlysame(A::AbstractArray, B::AbstractArray; atol = 0.0) = all(map((a,b,atol)-> nearlysame(a, b, atol), A, B, atol))
I think this would be better
img = [0 0 0 0 0 | ||
0 0 1 0 0 | ||
0 1 0 0 0 | ||
0 0 1 0 0 | ||
0 0 0 0 0] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we would also want to test OffsetArrays and Gray/RGB images.
Fixes: #133
Doubt: