Enh: Simplify controlling text of ImageInspectorOverlay

[jonathanrocher]

• Refactor ImageInspectorOverlay._new_value_updated to add a new method ImageInspectorOverlay._build_text_from_event to build the text to display, so it is easier to subclass and customize. Feedback on new method name welcome.
• Added a few unit tests for the tool and overlay since nothing existed.

Unrelated: the unit tests pointed out some tool behavior that doesn't make sense to me: if hasattr(plot, "_cached_mapped_image") is False, the data value is collected with key color_value instead of data_value. Does that seem wrong to anyone else?

[jonathanrocher]

Ready for feedback/review. cc @corranwebster @jwiggins

[jvkersch]

LGTM, just one request to change the test file name.

[jvkersch]

if hasattr(plot, "_cached_mapped_image") is False, the data value is collected with key color_value instead of data_value. Does that seem wrong to anyone else?

Where exactly does this happen?

[jonathanrocher]

if hasattr(plot, "_cached_mapped_image") is False, the data value is collected with key color_value instead of data_value. Does that seem wrong to anyone else?

Where exactly does this happen?

Compare line 78 to line 71 of chaco/tools/image_inspector_tool.py in the new version in this PR. The data value is being stored with key color_value. For consistency with the other if case, I would expect that data to be stored in data_value, but that wouldn't be backward compatible.

[jonathanrocher]

@jvkersch Thanks for reviewing my PR. I made the file name change and this is ready for a second look.

[jvkersch]

Compare line 78 to line 71 of chaco/tools/image_inspector_tool.py in the new version in this PR. The data value is being stored with key color_value.

Ah, thanks for pointing that out. That is almost certainly a bug... I agree with changing it to use data_value for the key -- I cannot imagine how this would have been functional otherwise.