Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

port restrict from Images #59

Merged
merged 1 commit into from
Aug 20, 2021
Merged

port restrict from Images #59

merged 1 commit into from
Aug 20, 2021

Conversation

johnnychen94
Copy link
Member

@johnnychen94 johnnychen94 commented Aug 20, 2021

With restrict now moved to ImageBase, it's possible to port the glue codes from Images.jl to this package and ImageMetadata.

restrict(img::AxisArray, ::Tuple{}) = img

function restrict(img::AxisArray, region::Dims)
inregiont = ntuple(i->in(i, region), ndims(img))
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Originally this is:

-     inregiont = ntuple(i->in(i, region), ndims(img))
+     inregion = falses(ndims(img))
+     inregion[[region...]] .= true
+     inregiont = (inregion...,)::NTuple{ndims(img),Bool}

It's more efficient to use ntuple here:

imgcol = rand(RGB{Float64}, 5, 6)
imgcolax = AxisArray(imgcol, :y, :x)
@btime restrict($imgcolax, (1,2))
# before: 965.077 ns (18 allocations: 1.62 KiB)
# after:  690.381 ns (12 allocations: 1.33 KiB)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it's funny looking at old code I wrote! 😆 Thanks for cleaning this up.

Copy link
Member

@timholy timholy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I strongly support this! It's much better to have this at as low a level as possible.

ImageCore = "0.8.1, 0.9"
Reexport = "0.2, 1.0"
SimpleTraits = "0.8, 0.9"
julia = "1"

[extras]
ColorVectorSpace = "c3611d14-8923-5661-9e6a-0046d554d3a4"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ColorVectorSpace is already loaded by ImageCore 0.9. Perhaps we should just standardize on 0.9? Or is it transitionally useful to bridge the gap? (I'm fine with this if so, it's a very minor point.)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I just planned to set all our packages on top of ImageCore 0.9 and drop compatibilities with old versions JuliaImages/Images.jl#972.

Will do this in the next PR.

restrict(img::AxisArray, ::Tuple{}) = img

function restrict(img::AxisArray, region::Dims)
inregiont = ntuple(i->in(i, region), ndims(img))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it's funny looking at old code I wrote! 😆 Thanks for cleaning this up.

@johnnychen94 johnnychen94 merged commit 6bb8fa4 into master Aug 20, 2021
@johnnychen94 johnnychen94 deleted the jc/restrict branch August 20, 2021 09:30
johnnychen94 added a commit to JuliaImages/Images.jl that referenced this pull request Oct 31, 2021
We used to host `restrict` in ImageTransformations, which is a big dependency
for ImageAxes and ImageMetadata. Thus specialized `restrict` methods for those
data types are kept in Images. Now that we have `restrict` in ImageBase, we can
move these specialized methods to ImageAxes and ImageMetadata.

This commit requires three new versions:

- ImageBase v0.1.2: this version fixes some of old `restrict` type inferrence issues.
- ImageAxes v0.6.9: JuliaImages/ImageAxes.jl#59
- ImageMetadata v0.9.7: JuliaImages/ImageMetadata.jl#79

AxisArrays is no longer needed because the method is moved to ImageAxes.

ImageTransformations <v0.9 compatibility is dropped so that we can avoid
symbol conflict on `restrict`.
johnnychen94 added a commit to johnnychen94/Images.jl that referenced this pull request May 21, 2022
We used to host `restrict` in ImageTransformations, which is a big dependency
for ImageAxes and ImageMetadata. Thus specialized `restrict` methods for those
data types are kept in Images. Now that we have `restrict` in ImageBase, we can
move these specialized methods to ImageAxes and ImageMetadata.

This commit requires three new versions:

- ImageBase v0.1.2: this version fixes some of old `restrict` type inferrence issues.
- ImageAxes v0.6.9: JuliaImages/ImageAxes.jl#59
- ImageMetadata v0.9.7: JuliaImages/ImageMetadata.jl#79

AxisArrays is no longer needed because the method is moved to ImageAxes.

ImageTransformations <v0.9 compatibility is dropped so that we can avoid
symbol conflict on `restrict`.
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants