-
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
Move points_in_polygon to Cython #250
Conversation
This function was part of the agg C-extension. It has no dependence on anything in AGG, so it can be moved into its own extension.
Current coverage is 52.08% (diff: 100%)@@ master #250 diff @@
==========================================
Files 126 127 +1
Lines 13176 13180 +4
Methods 0 0
Messages 0 0
Branches 1943 1941 -2
==========================================
+ Hits 6859 6865 +6
+ Misses 5872 5870 -2
Partials 445 445
|
Generally a good idea, although I haven't tried this out. |
This is a backwards-incompatible change which will require updating some stuff in Chaco (and at least one other internal project I know of). Possibly this should be kept in kiva (although not in kiva.agg), rather than moved to enable? I'm in two minds. |
I'm planning for |
While this is currently used for hit detection and point selection by Chaco, this is the sort of routine that can be useful for higher-level drawing routine optimization (eg. avoiding drawing points which are obscured). In other words, although it's not about drawing, it is about pure geometry, which seems a more Kiva-y thing than an Enable-y thing. |
Agreed. I've moved the code back to kiva, but still outside of agg. |
Thanks for reviewing! |
@jwiggins Can you please expose it back in |
Sure thing |
Since I'm maneuvering to eliminate the agg extension from kiva, I need to do something with
points_in_polygon
. I believe the only reason it was part of that extension was to cut down on the number of extension modules being built. Now it lives in its own extension.NOTE: I don't want to merge this until after 4.6.0 is released.