Skip to content
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

permuteddimsview -> PermutedDimsArray #143

Merged
merged 9 commits into from
Sep 28, 2020

Conversation

vtjeng
Copy link
Contributor

@vtjeng vtjeng commented Sep 27, 2020

Summary

permutteddimsview is deprecated.

Base.@deprecate_binding permuteddimsview PermutedDimsArray

We remove all references to this function (other than in tests specifically for deprecation), replacing it with PermutedDimsarray instead.

As part of this work, we re-ran the code in docs/src/views.md and updated it.

In particular, we remove a section that is now irrelevant ("A note on the return types from the views"). Here is the code that was written for that section:

Updated

Return types from views

DocTestSetup = quote
    using Colors, ImageCore
end

Consider the following examples with different input images.

In both instances, using the channelview returns a reinterpret
(but with different type parameters).

julia> img = [RGB(1,0,0) RGB(0,1,0);
              RGB(0,0,1) RGB(0,0,0)]
2×2 Array{RGB{N0f8},2} with eltype RGB{Normed{UInt8,8}}:
 RGB{N0f8}(1.0,0.0,0.0)  RGB{N0f8}(0.0,1.0,0.0)
 RGB{N0f8}(0.0,0.0,1.0)  RGB{N0f8}(0.0,0.0,0.0)

julia> cv = channelview(img)
3×2×2 reinterpret(N0f8, ::Array{RGB{N0f8},3}):
[:, :, 1] =
 1.0  0.0
 0.0  0.0
 0.0  1.0

[:, :, 2] =
 0.0  0.0
 1.0  0.0
 0.0  0.0
julia> img0 = rand(RGB{Float64}, 3, 2)
3×2 Array{RGB{Float64},2} with eltype RGB{Float64}:
 RGB{Float64}(0.721651,0.436883,0.0249213)  RGB{Float64}(0.838438,0.741055,0.289334)
 RGB{Float64}(0.314968,0.175824,0.392317)   RGB{Float64}(0.984615,0.186958,0.328459)
 RGB{Float64}(0.982128,0.922775,0.704911)   RGB{Float64}(0.967755,0.488064,0.780718)

julia> imgs = view(img0, 1:2:3, :)
2×2 view(::Array{RGB{Float64},2}, 1:2:3, :) with eltype RGB{Float64}:
 RGB{Float64}(0.721651,0.436883,0.0249213)  RGB{Float64}(0.838438,0.741055,0.289334)
 RGB{Float64}(0.982128,0.922775,0.704911)   RGB{Float64}(0.967755,0.488064,0.780718)

julia> channelview(imgs)
3×2×2 reinterpret(Float64, reshape(view(::Array{RGB{Float64},2}, 1:2:3, :), 1, 2, 2)):
[:, :, 1] =
 0.721651   0.982128
 0.436883   0.922775
 0.0249213  0.704911

[:, :, 2] =
 0.838438  0.967755
 0.741055  0.488064
 0.289334  0.780718

@vtjeng vtjeng changed the title permuteddimsview -> PermutedDimsarray permuteddimsview -> PermutedDimsArray Sep 27, 2020
@codecov
Copy link

codecov bot commented Sep 27, 2020

Codecov Report

Merging #143 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #143   +/-   ##
=======================================
  Coverage   55.14%   55.14%           
=======================================
  Files          10       10           
  Lines         457      457           
=======================================
  Hits          252      252           
  Misses        205      205           

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 c4d3bc9...92d830b. Read the comment docs.

@vtjeng
Copy link
Contributor Author

vtjeng commented Sep 27, 2020

@timholy It looks like I'm failing several unit tests here that aren't related to the code change. Specifically, starting at https://github.com/JuliaImages/ImageCore.jl/pull/143/checks?check_run_id=1173227622#step:5:318,

All errors:
===========================================
ArgumentError: Package ImageIO not found in current path:
- Run `import Pkg; Pkg.add("ImageIO")` to install the ImageIO package.

===========================================
ArgumentError: Package QuartzImageIO not found in current path:
- Run `import Pkg; Pkg.add("QuartzImageIO")` to install the QuartzImageIO package.

===========================================

UndefVarError: libwand not defined
===========================================
Fatal error:
4D inputs: Error During Test at /Users/runner/work/ImageCore.jl/ImageCore.jl/test/views.jl:235
  Got exception outside of a @test
  ArgumentError: Package ImageIO not found in current path:
  - Run `import Pkg; Pkg.add("ImageIO")` to install the ImageIO package.
...

Do you know what's going on here?

@johnnychen94
Copy link
Member

johnnychen94 commented Sep 27, 2020

This is due to the recent changes in BinaryBuilder and artifacts in julialang/julia, see also JuliaIO/ImageMagick.jl#189

I've disabled fail-fast in 16c688c, now I need to close and reopen to retrigger the test on the new base.

Copy link
Member

@johnnychen94 johnnychen94 left a comment

Choose a reason for hiding this comment

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

Thanks for updating this.

I guess this documentation was written when @example feature is not yet implemented, would you mind to upgrade the code blocks with @example and jldoctest things? That could indeed help capture such kind of outdated docs in the future.

docs/src/views.md Show resolved Hide resolved
@vtjeng
Copy link
Contributor Author

vtjeng commented Sep 27, 2020

Okay, I've updated this and tested the doctests locally with

cd docs
julia --project --color=yes make.jl

This should be ready for another review.

@johnnychen94 johnnychen94 merged commit 18f690f into JuliaImages:master Sep 28, 2020
@johnnychen94
Copy link
Member

@vtjeng thank you very much for these updates!

@vtjeng vtjeng deleted the remove-deprecated-function branch September 28, 2020 17:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants