chaco icon indicating copy to clipboard operation
chaco copied to clipboard

Published 20 hours ago • verified publisher icon enthought

Allow bgcolor to be set to any color, not just string

Open kjordahl opened this issue 11 years ago • 10 comments

Setting the bgcolor trait of a Plot instance (or an instance of its parent DataView class) only works if the argument is a string. This change allows it to take any color value. This also adds a colors_equal() function to chaco.api to allow comparison of colors that may be in different formats.

Without this change, the following will fail, raising KeyError:

from chaco.api import Plot
plot = Plot(bgcolor=(1, 1, 1))

Tests that cover the new function and the bgcolor bug are included.

In general, colors should be settable to any ColorTrait allowed value, not just strings. There are a few more comparisons in the codebase that could be cleaned up to allow more general color definitions as well.

kjordahl avatar Feb 03 '14 03:02 kjordahl

Can one of the admins verify this patch?

enbuilder avatar Feb 03 '14 03:02 enbuilder

please test this

prabhuramachandran avatar Feb 03 '14 07:02 prabhuramachandran

test this please

prabhuramachandran avatar Feb 03 '14 07:02 prabhuramachandran

ok to test

prabhuramachandran avatar Feb 03 '14 08:02 prabhuramachandran

Travis says OK

kjordahl avatar Feb 03 '14 12:02 kjordahl

Yes there is one Linux test failure. It is green on OS X and Windows (both 32 and 64bit). The failure is in: chaco.tests.border_test_case.DrawBorderTestCase.test_draw_border_simple

prabhuramachandran avatar Feb 03 '14 12:02 prabhuramachandran

This was a test setup error and is now fixed. All is well on all 6 platforms.

prabhuramachandran avatar Feb 03 '14 13:02 prabhuramachandran

LGTM. Merging tomorrow if there are no objections.

jonathanrocher avatar Feb 07 '14 04:02 jonathanrocher

We should finish and close this PR... I haven't had the time to help. @kjordahl do you think you will have some time soon?

jonathanrocher avatar Mar 20 '14 02:03 jonathanrocher

@jonathanrocher Thanks for the ping, obvioussly this had dropped off my radar. I'll try to pick it up again.

kjordahl avatar Mar 20 '14 13:03 kjordahl