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

deprecate ColorizedArray in favor of MappedArray #927

Merged
merged 1 commit into from
Sep 2, 2021

Conversation

johnnychen94
Copy link
Member

@johnnychen94 johnnychen94 commented Nov 22, 2020

MappedArray provides a more generic version of ColorizedArray so I believe it is good to not maintain ColorizedArray anymore.

LazyArray also provides similar operation; LazyArray(@~ @. I * indexed_img).

closes JuliaImages/juliaimages.github.io#132

@johnnychen94 johnnychen94 requested a review from timholy November 22, 2020 21:37
@codecov
Copy link

codecov bot commented Nov 22, 2020

Codecov Report

Merging #927 (17ab0e1) into master (cc578b0) will increase coverage by 0.04%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #927      +/-   ##
==========================================
+ Coverage   86.20%   86.24%   +0.04%     
==========================================
  Files           9        9              
  Lines        1022     1025       +3     
==========================================
+ Hits          881      884       +3     
  Misses        141      141              
Impacted Files Coverage Δ
src/labeledarrays.jl 100.00% <100.00%> (ø)

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 cc578b0...17ab0e1. Read the comment docs.

@timholy
Copy link
Member

timholy commented Nov 23, 2020

The one thing I worry about is ImageView. If I remember correctly, hover-over-pixel for a ColorizedArray displays the underlying value, not the mapped value. But we can do this if you think it's better.

@johnnychen94
Copy link
Member Author

If I remember correctly, hover-over-pixel for a ColorizedArray displays the underlying value, not the mapped value.

I see. It does give some visual aids, but this doesn't sound like what ColorizedArray is designed for. Probably making hover-over-pixel behavior customizable is a better choice. I don't use ImageView but I guess it's similar to JuliaImages/ImageView.jl#228.

@timholy
Copy link
Member

timholy commented Nov 23, 2020

this doesn't sound like what ColorizedArray is designed for

Not sure I really remember, but I think that is its main purpose. But you're right there are other ways to solve this. If this just seems too marginal we can do it, I was just thinking that there are many cases where array content and array labeling are two things that are useful to be able to link without losing track of either one.

@johnnychen94
Copy link
Member Author

johnnychen94 commented Nov 23, 2020

I get your point here. Given that you are probably the only user of this, I'll leave this to you to decide whether we should do it or not.

BTW, I searched the whole GitHub and couldn't find any other place except Images.jl that contains ColorizedArray 😨

@johnnychen94 johnnychen94 added this to the v1.0 milestone Aug 28, 2021
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.

[RFC] deprecate ColorizedArray and IndirectArray
2 participants