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

Provide hints about how to handle InexactErrors during saving #18

Merged
merged 1 commit into from
Dec 20, 2015

Conversation

timholy
Copy link
Member

@timholy timholy commented Dec 9, 2015

Currently we have this:

julia> A = [-1.0 0.0; 0.5 1.0]
2x2 Array{Float64,2}:
 -1.0  0.0
  0.5  1.0

julia> using FileIO

julia> save("/tmp/test.png", A)
WARNING: InexactError()
 in trunc at float.jl:357
 in _map_a! at /home/tim/.julia/v0.4/Images/src/map.jl:489
 in image2wand at /home/tim/.julia/v0.4/ImageMagick/src/ImageMagick.jl:151
 in save_ at /home/tim/.julia/v0.4/ImageMagick/src/ImageMagick.jl:146
 in save at /home/tim/.julia/v0.4/ImageMagick/src/ImageMagick.jl:64
 in save at /home/tim/.julia/v0.4/FileIO/src/loadsave.jl:92
 in save at /home/tim/.julia/v0.4/FileIO/src/loadsave.jl:51

which is pretty cryptic (see JuliaImages/Images.jl#422).

This adds a helpful hint:

WARNING: Mapping to the storage type failed; perhaps your data had out-of-range values?
Try `map(Images.Clamp01NaN(img), img)` to clamp values to a valid range.

It's a little verbose to include the module name, but if people are working with plain arrays then I was concerned that Clamp01NaN might not be in-scope.

What do you think, @xanderdunn?

@SimonDanisch
Copy link
Member

That's a lot better! :) Maybe we should start introducing InvalidData exceptions, which indicate that it's expected that no loader will save correctly.
I think the exception handler in FileIO needs a revamp anyways. I find the Warning --> Exception(same_warning) pretty annoying.
Also, has there been a discussion already, if we can have more modes to clamp by default? I run into this when doing some arithmetic with colors.
It's really annoying, that you have to clamp colors yourself all the time ;) I basically ended up defining -,+,* which do clamping after the arithmetic. This was arithmetic on U8, though, which is probably not that common...

@timholy
Copy link
Member Author

timholy commented Dec 20, 2015

@xanderdunn, I'm going to assume that you'd find this helpful.

timholy added a commit that referenced this pull request Dec 20, 2015
Provide hints about how to handle InexactErrors during saving
@timholy timholy merged commit 835a2d6 into master Dec 20, 2015
@timholy timholy deleted the teh/clamp_errormessages branch December 20, 2015 13:15
@timholy
Copy link
Member Author

timholy commented Dec 20, 2015

Also, has there been a discussion already, if we can have more modes to clamp by default? I run into this when doing some arithmetic with colors.
It's really annoying, that you have to clamp colors yourself all the time ;) I basically ended up defining -,+,* which do clamping after the arithmetic.

The problem with the latter strategy is that if you define a clamping +, then
c = (a+b)/2 may saturate, and then go to half-saturation, for no good reason. AFAICT the only way to get sensible behavior is for the programmer to specify when clamping should occur. See also JuliaGraphics/Colors.jl#7

I'm not exactly sure what you mean by "more modes"? Do you mean something besides clamping to the interval [0,1]?

@SimonDanisch
Copy link
Member

I'm not exactly sure what you mean by "more modes"?

I really didn't express myself very well, since it wasn't that thought out whatsoever!
I basically mean modes, to make this easier:

AFAICT the only way to get sensible behavior is for the programmer to specify when clamping should occur

like a series of functions, e.g. clamped_plus, clamped_minus. This wouldn't be very elegant, though.
clamp(a+b) seems nicer in comparision. So maybe everything is fine already!

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