Fixed bad vector transforms. - #2444
Conversation
There was a problem hiding this comment.
@mdboom - do you know which form is the correct one for the collections offsets transform? Notice that this is different to the quiver one - I'm guessing there is something special about either Quiver or QuadMesh which suggests one of them hasn't implemented the correct Collection API...
There was a problem hiding this comment.
P.S. That is not something I wanted to fix in this PR - I was actually trying to limit the scope 😄
There was a problem hiding this comment.
I'm not sure. I wonder if removing get_datalim altogether here (and letting it fall through to the base class) wouldn't also have the correct result...
There was a problem hiding this comment.
That is the best review action I've ever had - "delete this and see if it works" - and it did! Hurrah 👍
|
Looks like this makes an image test fail on the travis machine (not on mine though) and the upload has failed. Any ideas @mdboom? |
|
I think I broke the Travis uploading when I tried to move it to a script. I think Travis (presumably for security reasons) doesn't provide those environment variables to scripts. I've moved it back on v1.3.x and master. If you rebase this on the current v1.3.x it might fix it (I haven't verified my theory, but this will be a good test of it.) |
|
Looks like pickling broke |
Did you get a look at the differences? It doesn't fail on my machine. |
|
No, I was just reporting what Travis said failed. |
|
In addition, the uploading of results to S3 appears to still be broken. I'm looking into it. |
|
I forgot: The uploading of test results doesn't work on pull requests. The reason for this is that anyone can submit a pull request and "steal" the Amazon S3 keys, whereas those that can commit code to the main repository are at least theoretically trusted 😉. To get around this, @pelson since you have commit rights to the main repository, you could push your branch to a "staging" branch in the main repo and then monitor Travis for the results, for example: |
|
Thanks Mike. Will do. |
@mdboom - Any ideas why my bz2 file is only 46 bytes? |
|
Sorry -- I left an errant |
|
Ok, I've managed to find that this is related to the plot limits of the pickle test. I've tested the appropriate method with the same data in test_collections, and removed the autolimit for the plot in the pickle test. |
|
Travis doesn't seem to be picking up the latest commit for this PR... |
|
@pelson - this one seems close, can it be ready before 1.4.0rc1? |
|
I can't download the tar file @mdboom - any ideas? |
|
It seems my AWS account has been deleted. I'll have to investigate further... |
|
Sorry for the false alarm -- my S3 account was under a different e-mail than I was trying. The issue (and I had to jog my memory here, of course), is that the uploading of test results doesn't work on Pull Requests, for security reasons (otherwise anybody could file a pull request and steal my Amazon credentials). I realize that makes the whole thing less than useful. I should add a message to the log about this so we don't forget in the future. I believe, however, since you and I have commit rights to the main repo, we can push this branch to a temporary branch in the main repo (e.g. "staging") and Travis should test it. At least I tried that [1] -- let's see if it works: |
There was a problem hiding this comment.
Looks like the failure is due to the y-extent of the streamplot plot. Might you need to set these limits explicitly for the streamplot case too?
|
Thanks @mdboom - I've also tidied up the "staging" branch. |
Fixed bad vector transforms.
5 participants
I've finally got around to looking at vector transforms in mpl (obviously for Cartopy functionality) and it turns out that quiver/barbs/streamplot all suffer from an almost identical issue with the way they handle the bounding box updates. This bug fix opens up the
get_datalimmethod so that all Collections can override their own behaviour without needing special cases for each of the datatypes.This has come from a ticket over at cartopy: SciTools/cartopy#351
Note this PR is against v1.3.x.