This will prevent the issues Blake encountered from happening again.
http://groups.google.com/group/ocio-dev/browse_thread/thread/c073e0f4fde995f7

This also addresses an outstanding issue:
https://github.com/imageworks/OpenColorIO/issues#issue/17

* Configs get a 'sanityCheck' call, which will validate the
configuration and throw an exception if any errors are detected. An
example of an error condition would be a role reference to a
colorspace that does not exist. Repeated calls to sanityCheck are
cached (and fast).

* Nuke plugins call sanityCheck() before the getProcessor() call.

* getColorSpace call is now not sensitive to case differences. Aka,
linear == LINEAR.

* The default role is only returned as the colorspace of 'last resort'
if strict_parsing is explicitly disabled. This will prevent
colorspace errors to silently roll by, unless requested.

Commits:
2ae25fc437
993773f9ad
3607e94d29

-- Jeremy

Your idea would also work. Not sure if either is better.

sanityCheck() was inspired by OpenEXR, which has a function of the
same name, and performs a similar action. Guess I just had that model
in mind when writing this.

Are you against exceptions?

If we went with your idea, why an enum instead of a bool?
"bool isValid()"

I would expect users to only want to know if there is an error or not.
Under what circumstances would a user be returned the "UNKNOWN" value?
The reason I have 3 states internally is to know if i've computed the
value, and if so, if it succeeded or not. The internal 'unknown'
state is just an implementation detail, the user is not aware of it.

-- Jeremy

Either of our approaches will work, of course, but I'm still inclined
to go with the exception route.

A bunch of calls in OCIO can throw exceptions (the common one being
config->getProcessor(...) ), so most of the time you'll already have
try / catch blocks. You can usually thus add a "sanityCheck" call in
a single extra line.

If we instead added: bool isValid(const char * msg)
this requires a separate code block to explicitly check it, probably 3
or 4 lines of code. Which isn't a problem, just an observation.

I'm also not the biggest fan of functions with two return arguments.
Consider the python API... If we go the exception route, it ports
naturally. If we use an isValid call, would it return the tuple
"(bool, str)"? People would probably expect it to just return a bool.

Perhaps you object to the name 'sanityCheck'? (Which I admit is a bit
weird). How about 'validate', instead?

OCIO::GetCurrentConfig() will always return a valid profile object.
It currently will throw an exception only iff $OCIO is defined, but
points to a profile that has parse problems.

I'd prefer to not enforce 'auto validation' after loading at the library level.

-- Jeremy

On 23/12/2010, at 5:38 AM, Jeremy Selan wrote:

Either of our approaches will work, of course, but I'm still inclined
to go with the exception route.

A bunch of calls in OCIO can throw exceptions (the common one being
config->getProcessor(...) ), so most of the time you'll already have
try / catch blocks. You can usually thus add a "sanityCheck" call in
a single extra line.

If we instead added: bool isValid(const char * msg)
this requires a separate code block to explicitly check it, probably 3
or 4 lines of code. Which isn't a problem, just an observation.

I'm also not the biggest fan of functions with two return arguments.
Consider the python API... If we go the exception route, it ports
naturally. If we use an isValid call, would it return the tuple
"(bool, str)"? People would probably expect it to just return a bool.

yeah I guess you would either need to do that or have some config.getErrorMsg() function.

Perhaps you object to the name 'sanityCheck'? (Which I admit is a bit
weird). How about 'validate', instead?

No I think it's ok esp if people are used to the ilm code.

The ilm code does call header.sanityCheck when using InputFile (), OutputFile(), TiledInputFile(), TiledOutputFile().

OCIO::GetCurrentConfig() will always return a valid profile object.
It currently will throw an exception only iff $OCIO is defined, but
points to a profile that has parse problems.

I'd prefer to not enforce 'auto validation' after loading at the library level.

I would say throwing inside sanityCheck is fine then.

If you think having a pointer to a profile which has problems is needed for something else then not throwing from in GetCurrentConfig() is also ok.

.malcolm

Thanks, I'm going to check this in then.

-- Jeremy

On Wed, Dec 22, 2010 at 3:07 PM, Malcolm Humphreys
<malcolmh...@...> wrote:

On 23/12/2010, at 5:38 AM, Jeremy Selan wrote:

Either of our approaches will work, of course, but I'm still inclined
to go with the exception route.

A bunch of calls in OCIO can throw exceptions (the common one being
config->getProcessor(...) ), so most of the time you'll already have
try / catch blocks.  You can usually thus add a "sanityCheck" call in
a single extra line.

If we instead added: bool isValid(const char * msg)
this requires a separate code block to explicitly check it, probably 3
or 4 lines of code.   Which isn't a problem, just an observation.

I'm also not the biggest fan of functions with two return arguments.
Consider the python API... If we go the exception route, it ports
naturally.   If we use an isValid call, would it return the tuple
"(bool, str)"?  People would probably expect it to just return a bool.

yeah I guess you would either need to do that or have some config.getErrorMsg() function.

Perhaps you object to the name 'sanityCheck'? (Which I admit is a bit
weird).  How about 'validate', instead?

No I think it's ok esp if people are used to the ilm code.

The ilm code does  call header.sanityCheck when using InputFile (), OutputFile(), TiledInputFile(), TiledOutputFile().

OCIO::GetCurrentConfig() will always return a valid profile object.
It currently will throw an exception only iff $OCIO is defined, but
points to a profile that has parse problems.

I'd prefer to not enforce 'auto validation' after loading at the library level.

I would say throwing inside sanityCheck is fine then.

If you think having a pointer to a profile which has problems is needed for something else then not throwing from in GetCurrentConfig() is also ok.

.malcolm