Update IntegralArrays.jl #3
Conversation
It looks like the code for the integral array is outdated (actually, removed). Here's a possibility for this code, which I wrote for my face detection algorithm. I suspect there are things that can be made much better, but this is a start.
There was a problem hiding this comment.
Thanks for doing this!
I do wonder, though, if it would make sense just to just cumsum and cumsum!. Basically I think the only thing you'd have to add would be looping over dimensions. You could even let the user select which dimensions the summation occurs over, defaulting to "all".
src/IntegralArrays.jl
Outdated
| | . . . . | . . . . . | ||
| =# | ||
|
|
||
| function getImageMatrix(imageFile::AbstractString) |
There was a problem hiding this comment.
We follow the Julia style guide and will probably transition to the Blue style guide as well, so please using naming consistent with standard Julia choices.
src/IntegralArrays.jl
Outdated
| =# | ||
|
|
||
| function getImageMatrix(imageFile::AbstractString) | ||
| img = load(imageFile) |
There was a problem hiding this comment.
I am not sure we need IO functionality in this library. There's nothing here specific to an integral array representation.
src/IntegralArrays.jl
Outdated
|
|
||
| function toIntegralImage(imgArr::AbstractArray) | ||
|
|
||
| arrRows, arrCols = size(imgArr) # get size only once in case |
There was a problem hiding this comment.
Note this assumes 2d. While not fatal, most of the algorithms in JuliaImages support 3d images too. Julia makes it easy to use the same code for arbitrary dimensionality.
src/IntegralArrays.jl
Outdated
|
|
||
| arrRows, arrCols = size(imgArr) # get size only once in case | ||
| rowSum = zeros(arrRows, arrCols) | ||
| integralImageArr = zeros(arrRows, arrCols) |
There was a problem hiding this comment.
This assumes grayscale but the input arguments are general.
src/IntegralArrays.jl
Outdated
| integralImageArr = zeros(arrRows, arrCols) | ||
|
|
||
| # process the first column | ||
| for x in 1:(arrRows) |
There was a problem hiding this comment.
| for x in 1:(arrRows) | |
| for x in 1:arrRows |
However, to support arbitrary axes (https://julialang.org/blog/2017/04/offset-arrays/) it's best to say for x in axes(imgArr, 1)
src/IntegralArrays.jl
Outdated
| # same as above: we cannot access a 0th element in the matrix | ||
| # our scalar accumulator s will catch the 1st row, so we only | ||
| # needed to predefine the first column before this loop | ||
| if isone(y) |
There was a problem hiding this comment.
Here it would be easier to use for y in 2:arrCols, or for more general axes for y in Iterators.drop(axes(y, 2), 1).
This is my best attempt at adapting #1. What do you think?
|
I updated #1 to work for 1.5. What do you think? You undoubtedly know much more Julia than I, so please let me know what still needs to be changed. Including (and especially) regarding code style and convention 😄 |
|
Hi @jakewilliami, sorry for the long delay here. In general when you're updating someone else's PR, it's best to start from their branch so you can preserve the attribution of their contribution. I merged #1 and credited you as a co-author of my changes. Thanks for helping get this over the finish line! |
It looks like the code for the integral array is outdated (actually, removed). Here's a possibility for this code, which I wrote for my face detection algorithm. I suspect there are things that can be made much better, but this is a start.