JuliaImages / ImageFiltering.jl

Julia implementations of multidimensional array convolution and nonlinear stencil operations
Other
99 stars 51 forks source link

fix freqkernel #207

Closed JeffFessler closed 3 years ago

JeffFessler commented 3 years ago

Addresses #206 The substance here is the changed lines with wrapindex and the corresponding added test. There is also a subtle point that the kernel needs to be supported in [-N/2, N/2-1) so I added checks for that too (handling both even and odd N cases). And some minor tweaking of docstrings.

FWIW, I'm teaching a graduate image processing course where I am trying to translate everything from Matlab to Julia so getting this psf2tof equivalent working is very desirable.

codecov[bot] commented 3 years ago

Codecov Report

Merging #207 (e57a5b3) into master (b4f3b7b) will increase coverage by 0.03%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #207      +/-   ##
==========================================
+ Coverage   91.07%   91.11%   +0.03%     
==========================================
  Files          10       10              
  Lines        1412     1418       +6     
==========================================
+ Hits         1286     1292       +6     
  Misses        126      126              
Impacted Files Coverage Δ
src/ImageFiltering.jl 77.27% <ø> (ø)
src/mapwindow.jl 86.28% <100.00%> (+0.06%) :arrow_up:
src/utils.jl 88.23% <100.00%> (+0.93%) :arrow_up:

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 b4f3b7b...e57a5b3. Read the comment docs.

JeffFessler commented 3 years ago

Hi @johnnychen94 are you a maintainer here? This PR has been ready for a while. Anything else I should do?

JeffFessler commented 3 years ago

I just learned about Aqua.jl and tested it out on ImageFiltering.jl and it reported that ImageMetadata is unused so I removed it from deps. This is a bit off topic from freqkernel but figured I'd include it in this PR anyway and see how it goes...

JeffFessler commented 3 years ago

Also addressing #208 with a 1-line fix. Hopefully.

timholy commented 3 years ago

Very nice work, @JeffFessler. Thanks!

timholy commented 3 years ago

https://github.com/JuliaRegistries/General/pull/30822

johnnychen94 commented 3 years ago

I was AFK for the last two months so sorry for the inactivity. I've looked at the psf2otf source codes two years ago so I'm contaminated and thus can't review this PR (#81). Thank you both for the efforts.