Skip to content

Use draw_marker_at_points if possible otherwise use draw_path_at_points in ColormappedScatterPlot#672

Merged
aaronayres35 merged 5 commits into
masterfrom
fix/425-use-new-markers
Apr 19, 2021
Merged

Use draw_marker_at_points if possible otherwise use draw_path_at_points in ColormappedScatterPlot#672
aaronayres35 merged 5 commits into
masterfrom
fix/425-use-new-markers

Conversation

@aaronayres35
Copy link
Copy Markdown
Contributor

@aaronayres35 aaronayres35 commented Apr 15, 2021

fixes #425
This can close issue #232 as well after enthought/enable#782 is merged

In enable:
https://github.com/enthought/enable/blob/master/enable/markers.py
some markers have a defined kiva_marker while others simply define an _add_to_path method detailing how to draw the marker manually. If the marker has kiva_marker we can use draw_marker_at_points, but if it doesn't we need to fall back to manually drawing the path via the _add_to_path method and calling draw_path_at_points.

@aaronayres35 aaronayres35 changed the title [WIP] Use draw_marker_at_points if possible otherwise use draw_path_at_points in ColormappedScatterPlot Use draw_marker_at_points if possible otherwise use draw_path_at_points in ColormappedScatterPlot Apr 16, 2021
Comment on lines +90 to +96
# regression test for enthought/chaco#425
def test_non_kiva_marker(self):
self.scatterplot.marker = "star"

self.gc.render_component(self.scatterplot)
actual = self.gc.bmp_array[:, :, :]
self.assertFalse(alltrue(actual == 255))
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was being dumb before, it was trivial to write this regression test...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note this PR also fixes the test test_scatter_custom above once enthought/enable#782 is pushed through that test can be unskipped.

mode = marker_cls.draw_mode

if marker_cls != "custom":
if self.marker != "custom":
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

marker_cls is not a string, but the actual class e.g. <class 'enable.markers.CustomMarker'>


else:
path = marker_cls.custom_symbol
path = self.custom_symbol
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

custom_symbol is a trait on the ColormappedScatterPlot, not the marker. For example use, see test test_scatter_custom.


else:
path = marker_cls.custom_symbol
path = self.custom_symbol
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

custom_symbol is a trait on the ColormappedScatterPlot, not the marker. For example use, see test test_scatter_custom.

Copy link
Copy Markdown
Contributor

@rahulporuri rahulporuri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Do you want to open a separate issue about removing the test skip once we have a new enable?

@aaronayres35 aaronayres35 merged commit 149c76f into master Apr 19, 2021
@aaronayres35 aaronayres35 deleted the fix/425-use-new-markers branch April 19, 2021 12:26
@aaronayres35
Copy link
Copy Markdown
Contributor Author

I've opened #686

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.

new markers (star, hexagon...) available in ScatterPlot but not in ColormappedScatterPlot

2 participants