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

add a curried version of colorview #134

Merged
merged 2 commits into from
Aug 29, 2020
Merged

add a curried version of colorview #134

merged 2 commits into from
Aug 29, 2020

Conversation

johnnychen94
Copy link
Member

@johnnychen94 johnnychen94 commented Jul 4, 2020

The broken jldoctest will be fixed upstream in ImageInTerminal.

closes #132

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.

Great idea!

@codecov
Copy link

codecov bot commented Aug 28, 2020

Codecov Report

Merging #134 into master will decrease coverage by 4.68%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #134      +/-   ##
==========================================
- Coverage   60.50%   55.81%   -4.69%     
==========================================
  Files           9       10       +1     
  Lines         400      473      +73     
==========================================
+ Hits          242      264      +22     
- Misses        158      209      +51     
Impacted Files Coverage Δ
src/colorchannels.jl 30.64% <100.00%> (+0.56%) ⬆️
src/ImageCore.jl 50.00% <0.00%> (-6.00%) ⬇️
src/precompile.jl 0.00% <0.00%> (ø)
src/convert_reinterpret.jl 63.82% <0.00%> (+0.41%) ⬆️
src/deprecations.jl 65.21% <0.00%> (+65.21%) ⬆️

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 3beb222...2adbe2e. Read the comment docs.

Comment on lines +16 to +19
using ImageInTerminal
ImageInTerminal.disable_encoding()
doctest(ImageCore, manual = false)
ImageInTerminal.enable_encoding()
Copy link
Member Author

@johnnychen94 johnnychen94 Aug 28, 2020

Choose a reason for hiding this comment

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

The ideal and more elegant way I have in mind to do this is:

ReferenceTests.disable_encoding(:all) do
    doctest(ImageCore, manual = false)
end

But ReferenceTests is a little bit broken and fixing it requires more effort than I can put now.

@timholy I guess you're waiting for this PR to tag an ImageCore release? In this case, we could merge this version and clean up the direct test dependency of ImageInTerminal in the future. (ReferenceTests still requires ImageInTerminal)

@johnnychen94 johnnychen94 merged commit 11f0298 into master Aug 29, 2020
@johnnychen94 johnnychen94 deleted the jc/curried branch August 29, 2020 04:07
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.

Change (color)view arguments?
2 participants