patheffects for Line2d object #1015

Closed
wants to merge 3 commits into
from

Projects

None yet

3 participants

@leejjoon

This is rebased version of #655.
It also add initial patheffects support for Collections object.

@pelson pelson commented on the diff Jul 16, 2012
lib/matplotlib/collections.py
@@ -245,11 +247,21 @@ def draw(self, renderer):
if self._hatch:
gc.set_hatch(self._hatch)
- renderer.draw_path_collection(
- gc, transform.frozen(), paths, self.get_transforms(),
- offsets, transOffset, self.get_facecolor(), self.get_edgecolor(),
- self._linewidths, self._linestyles, self._antialiaseds, self._urls,
- self._offset_position)
+ if self.get_path_effects():
@pelson
pelson Jul 16, 2012

These bifurcations are a little concerning to me. Is it impossible that this behaviour could live in the renderer rather than having a path_effects _Base class?

@pelson
Matplotlib Developers member

I'm coming to this issue fresh, so I can't immediately see what benefit this PR brings - something being put in the changelog and the whats new page would be helpful.

@WeatherGod
Matplotlib Developers member

I think this has benefits, but I definitely agree that a note in docs/users/whats_new.rst is important. In addition, we need at least a new test for this feature.

@pelson
Matplotlib Developers member

I'm coming to this issue fresh, so I can't immediately see what benefit this PR brings

Apologies @leejjoon, re-reading that - it sounds worse than what I meant. What I was trying to convey was that its not clear, from an end user perspective, that a new feature has been made available on Line2D (no changelog, what's new etc.). I sincerely hope that hasn't discouraged you from moving this PR forwards.

@leejjoon - I'm still a little concerned by the bifurcation that I mentioned and wonder if it would be better to change the rendered API such that draw_path_collection takes the path effects (presumably, that way, an SVG renderer could make use of various styling without having to re-draw the line for each path effect).

@leejjoon - are you in a position to move this forwards any time soon?

Thanks,

Phil

@leejjoon

PathEffect is nothing new and I just extended this to Line2D. And unfortunately, I find less and less time to work on matpltolib, so not sure if I can further work on this. If you and others think this PR is not acceptable without changelog and tests, please just go ahead and close this.

I don't think changing the API is a good idea as same functionality need to be replicated in every backends. Maybe what we need is a refactoring of the draw method.

@leejjoon leejjoon closed this Apr 16, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment