Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

[WIP] Steppath and Line2D #1876

Closed
wants to merge 4 commits into
from

Conversation

Projects
None yet
4 participants
Contributor

Tillsten commented Apr 1, 2013

Ok, this is a first try to refactor Lines2D for steps. Submitted as a PR to run the tests. See #1709.

@Tillsten Tillsten closed this Apr 1, 2013

@Tillsten Tillsten reopened this Apr 1, 2013

Contributor

Tillsten commented Apr 1, 2013

So tests are still passing, any other concerns with this approach?

Contributor

Tillsten commented May 29, 2013

Ok, because i not sure if it is worth to continue this PR i am closing this.
I am still thinking that refactoring out the path calculation is the right think to do, but i am not so sure about the API change. Maybe just adding a get_drawn_path() is a better way.

@Tillsten Tillsten closed this May 29, 2013

Owner

mdboom commented May 29, 2013

@Tillsten: Other than the lack of response (for which I apologise -- the PR queue can be a real firehose sometimes), what's your reason for closing this? It seems reasonable...

Contributor

Tillsten commented May 29, 2013

@mdboom Mostly because i not sure if this is the right way to do it: the idea of the old design seems to be that the difference between draw_styles is only relevant for drawing.

The biggest practical concerns i have, is that i am not sure if line.set_data still works as as expected. The fact the test-suit works seems to imply it does, but i am still unwary. The return value of get_data now returns also something different (probably more corrected now), which is the aim of the PR.

Owner

tacaswell commented May 30, 2013

I would put a vote in for not having the function change the data so that set_[x,y]data/get_[x,y]data round trip works. Aside from my sense of symmetry, changing the data would mess up animation code where you append new data onto existing lines (by using get_[x,y]data -> append -> set_[x,y]data).

Contributor

Tillsten commented May 30, 2013

@tacaswell But the problem still remains, that at the moment you don't have any method to get the drawn data. Also the step-vertices are recalculated very draw.

Owner

tacaswell commented May 30, 2013

@Tillsten What do you mean by drawn data?

Is the calculation too slow to run every draw?

Contributor

Tillsten commented Jun 3, 2013

@tacaswell The problem that there is no way of getting the "real" vertices from a path which use steps. This problematic if you want to make a filled histogram. Speedwise: I not sure, i don't use mpl for a lot of interactive work because it is quite slow, so i didn't benchmark it. I think the difference will be only visible for a lot of step-plots.

Owner

tacaswell commented Jun 3, 2013

@Tillsten If you assume a bottom level, can you put in an axhline and then use fill_between?

@tacaswell No, see the link in the OP here: #1709 (it is old but still applies).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment