JuliaImages / ImageFiltering.jl

Julia implementations of multidimensional array convolution and nonlinear stencil operations

Other

99 stars 49 forks source link

add BorderArray #99

Closed jw3126 closed 5 years ago

jw3126 commented 5 years ago

98

codecov[bot] commented 5 years ago

Codecov Report

Merging #99 into master will increase coverage by 0.32%. The diff coverage is 90.24%.

@@            Coverage Diff             @@
##           master      #99      +/-   ##
==========================================
+ Coverage   81.12%   81.44%   +0.32%     
==========================================
  Files           8        9       +1     
  Lines        1118     1159      +41     
==========================================
+ Hits          907      944      +37     
- Misses        211      215       +4

Impacted Files	Coverage Δ
src/ImageFiltering.jl	`80% <ø> (ø)`	:arrow_up:
src/borderarray.jl	`90.24% <90.24%> (ø)`

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 3438a0c...e85d0bd. Read the comment docs.

codecov[bot] commented 5 years ago

Codecov Report

Merging #99 into master will increase coverage by 0.3%. The diff coverage is 88.15%.

@@            Coverage Diff            @@
##           master      #99     +/-   ##
=========================================
+ Coverage   81.12%   81.43%   +0.3%     
=========================================
  Files           8        9      +1     
  Lines        1118     1174     +56     
=========================================
+ Hits          907      956     +49     
- Misses        211      218      +7

Impacted Files	Coverage Δ
src/ImageFiltering.jl	`80% <ø> (ø)`	:arrow_up:
src/border.jl	`80.76% <77.77%> (-1.87%)`	:arrow_down:
src/borderarray.jl	`89.55% <89.55%> (ø)`
src/mapwindow.jl	`78.3% <0%> (+0.52%)`	: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 3438a0c...15187a0. Read the comment docs.

jw3126 commented 5 years ago

@timholy what do you think about the general design of BorderArray?

I did not implement setindex! since it would be mutating border + inner points simultaneously
What should happen with padarray? Should it stay as is? Should it return a BorderArray? Should it be removed? Personally I think it could be return a BorderArray and the current padarray should be spelled something like full(padarray(...)) instead.

timholy commented 5 years ago

This looks superficially awesome, but I haven't had bandwidth recently. Maybe I can review this tomorrow, sorry for the delay.

timholy commented 5 years ago

Once this passes tests, I approve merging. Thanks for this, fantastic work!

jw3126 commented 5 years ago

@timholy can we tag a new release?

timholy commented 5 years ago

Sorry I didn't notice the question about tagging. Absolutely! There's nothing breaking here, right? So this could be 0.6.1?

I think you should be able tag yourself, if you like. Perhaps give it a try, just so we know whether you are as empowered as you deserve to be.

jw3126 commented 5 years ago

Yeah, there is no breaking change, at least no intended one :smile:. I will see if I can create a release.