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 method to invert a ColorScheme (inverse of get) #11

Merged
merged 2 commits into from
Jan 20, 2018

Conversation

NHDaly
Copy link
Contributor

@NHDaly NHDaly commented Jan 17, 2018

Fixes #10.

This PR is to add a method which is the inverse of get(cscheme, x).

For now, I've just copied over invert from NHDaly/ColorSchemesInvert.jl. For more details see NHDaly/ColorSchemesInvert.jl/.../README.ipynb.

invert places a color within a colorscheme, by converting the color to
a value representing its position on the colorscheme's axis.

Put another way, it returns the value in the domain for which its
colorscheme value most closely matches the provided color.

`invert` places a color within a colorscheme, by converting the color to
a value representing its position on the colorscheme's axis.

Put another way, it returns the value in the domain for which its
colorscheme value most closely matches the provided color.

Also adds `convertToScheme(cscheme, img)`, which uses `invert` to
convert every pixel in img to its closest color from the provided
colorscheme.

Adds tests for both methods.
@NHDaly
Copy link
Contributor Author

NHDaly commented Jan 17, 2018

This is still a work in progress. I'm not exactly sure how the method should work, but this is a good start. It does what I think it should, although doesn't behave quite like I expected. See:
https://github.com/NHDaly/ColorSchemesInvert.jl/blob/4e50015b681679d45cf4f7ddf2d929cc63e67849/test/runtests.jl#L41

I'd love to talk it through some with folks! :)

Copy link
Member

@cormullion cormullion left a comment

Choose a reason for hiding this comment

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

The style is to use lowercase (“squishcase”, juliacase) rather than camelCase, so this should be convert_to_scheme or something better...

Copy link
Member

@cormullion cormullion left a comment

Choose a reason for hiding this comment

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

You could move the export line from line 5 to line 116.

Copy link
Member

@cormullion cormullion left a comment

Choose a reason for hiding this comment

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

I'm wondering whether an alternative to invert would be getinverse, because it doesn't invert some object, rather it 'gets' a value...

@NHDaly
Copy link
Contributor Author

NHDaly commented Jan 19, 2018

You could move the export line from line 5 to line 116.

Oh sure, can do. Thanks. I didn't notice it way down there. Is there no convention on putting the exports at the top?

@NHDaly
Copy link
Contributor Author

NHDaly commented Jan 19, 2018

I'm wondering whether an alternative to invert would be getinverse, because it doesn't invert some object, rather it 'gets' a value...

Yeah I think that's better. I can't really think of anything better than that. Thanks! I'll make that change too.

renamed methods; fixed broken convert_to_scheme definition; moved exports;
@NHDaly
Copy link
Contributor Author

NHDaly commented Jan 19, 2018

The style is to use lowercase (“squishcase”, juliacase) rather than camelCase, so this should be convert_to_scheme or something better...

Okay I've changed it to convert_to_scheme for now. But actually, I'm not even sure it's worth including in this PR at all.. It was mostly just something fun that occured to me after implementing getinverse. I think right now it has some surprising results due to a surprising definition of "similarity" between colors.

getinverse is very useful for determing where in a given ColorScheme a color lies -- if it's actually from that colorscheme, but it has unexpected effects when converting between color schemes. For example, here, one would expect the black-to-white scale to map 1:1 directly to the black-to-red, but this is not actually the case:

screen shot 2018-01-19 at 4 05 10 pm

So anyway, maybe we just remove this function for now?

@cormullion cormullion merged commit f0acf18 into JuliaGraphics:master Jan 20, 2018
@cormullion
Copy link
Member

Let’s merge this, make a release, and take it for a spin... If it proves problematic we can either document its problems or remove it; if it works we can improve the docs... it will all need extensive changes in a few months anyway!

@NHDaly NHDaly deleted the invert branch January 22, 2018 04:16
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.

Feature request: Invert a ColorScheme (inverse of get)
2 participants