-
Notifications
You must be signed in to change notification settings - Fork 44
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
Update custom marker as scale_ctm expects floats #782
Conversation
I don't see any unittests for the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Ref
enable/kiva/agg/src/kiva_compiled_path.h
Line 124 in a1a6059
void scale_ctm(double sx, double sy); |
I'd still like it if we could add a few tests.
👍 I've added 2 regression tests |
CI is failing with
perhaps this test just needs to be skipped on some combination of quartz / macos / wx? I am marking this as a WIP until I get a chance to look into this more |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still LGTM
from enable.compiled_path import CompiledPath | ||
from enable.kiva_graphics_context import GraphicsContext | ||
from enable.markers import CustomMarker | ||
from enable.tests._testing import skip_if_not_qt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not an actionable comment - i was wondering if the enable testsuite should always import from enable.api
instead of importing from the specific modules. Note that this is a more general question for ETS/package development.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I notice some of the test files do but others don't. I think it probably makes sense to use api everywhere in tests
This behavior of |
Needed to close enthought/chaco#232
Down in agg the method that gets called wants double inputs. If an int gets passed it things break. Changing the inputs to floats seems to resolve the problem.