patheffects for Line2d object #1015
Conversation
| offsets, transOffset, self.get_facecolor(), self.get_edgecolor(), | ||
| self._linewidths, self._linestyles, self._antialiaseds, self._urls, | ||
| self._offset_position) | ||
| if self.get_path_effects(): |
There was a problem hiding this comment.
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?
|
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. |
|
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. |
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 @leejjoon - are you in a position to move this forwards any time soon? Thanks, Phil |
|
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. |
This is rebased version of #655.
It also add initial patheffects support for Collections object.