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

Unhandled error with a decimal gamma value and input out of domain #331

Open
tibotiber opened this issue Mar 7, 2024 · 3 comments
Open
Labels

Comments

@tibotiber
Copy link

tibotiber commented Mar 7, 2024

Hello,

Before starting, let me say I love this library and have been relying on it for years. I'm even singing its praise in our own developer documentation: https://docs.smplrspace.com/api-reference/color/overview#a-little-context ;). So THANK YOU!

I've just come across what I think is a bug. If I use a scale with a decimal gamma value and pass it an out of domain input. It's crashing.

Code:

console.log(chroma.scale('YlGn').domain([5, 15]).gamma(1.2)(4).hex())

Error:

Uncaught Error: unknown format: 
    at new Color2 (chroma-js.js?v=8a5f8334:121:17)
    at chroma (chroma-js.js?v=8a5f8334:138:16)
    at f (chroma-js.js?v=8a5f8334:1998:19)

Happy to provide any additional details, testing, or help on a PR if given pointers of where to handle this.

Take care!

PS: this can be easily handled in userland by clamping the value before passing it in, so not a really bad one.

@regorxxx
Copy link

regorxxx commented Mar 10, 2024

Your code doesn't really make sense, since you are trying to map a value of 4 in a scale which goes from 5 to 15. Test the same with a value of 5 and it works,

Now, it's not the decimal part the problem in fact. Because if you use other values outside the map (like 200) it works, it's the pow function used to get the colors. i.e. the gamma is applied fine, it's only when you make scale(x) that it fails.

Pow throws NAN whenever the base is negative (below the domain) and the exponent is not integer. I have fixed it on my fork, since in such cases the values are always at the extreme left of the scale and could be clamped.

regorxxx added a commit to regorxxx/chroma.js that referenced this issue Mar 10, 2024
…ath.pow usage. See [Issue 331](gka#331)

Signed-off-by: regorxxx <regorxxx@protonmail.com>
@tibotiber
Copy link
Author

Thanks for the detailed explanation 🙂.

In case that helps, I think it's completely fair to define a scale and provide a value out of the domain. For example, you want to chart temperatures and consider 19 to 24 degree Celsius to be the effective range of visualization. If you get data that's at 18 degrees, maybe an outlier, I'd expect a the scale to clamp the value, not throw an error. Is that reasonable?

That's also how it works like 95% of the time, which is why I interpreted this as a bug.

@gka
Copy link
Owner

gka commented Jul 28, 2024

thank both @tibotiber and @regorxxx for raising the issue and the attempted fix. I agree that the outside domain use case is valid, so I'll mark this as bug and will take a look at the fix in regorxxx@3b85a96 soon.

@gka gka added the bug label Jul 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants