fix BboxConnectorPatch does not show facecolor #10686
Conversation
|
Thanks for working on this @zhoubecky ! Are we sure that this does not re-break any of the examples from #5757 ? Would you mind squashing this down to 1 commit? (http://gitready.com/advanced/2009/02/10/squashing-commits-with-rebase.html + |
627eb19 to
5781bd0
Compare
|
...also, I don't really understand the test plot. |
5781bd0 to
fa0df24
Compare
|
@tacaswell I have reverted a merge commit and squashing all my commits to 1 commit. |
|
@jklymak fields/parameters "color", "facecolor" and "fc" in BboxConnectorPatch and mark_inset didn't work before, what I am doing is to create a simple example using both BboxConnectorPatch and mark_inset, and test if those fields are working now. I test each field separately(so you can see three colorful bboxes and marked area), and I also test the "fill=False" scenario to see if color would be disable in this case. |
|
@zhoubecky unfortunately you also merged in a bunch of other changes that have been merged into master since then. Note your file count went way up. The trick is to squash your changes first and then rebase onto master (or re-order the commits when you WRT to the test, there is an extra colored box outside the axis. It all just looks strange. But if thats how its supposed to look, I guess its OK for a test. |
|
@jklymak are you talking about the two commits about style fixes? Should I revert those two commits now and only leave my commit in this branch? |
|
Yes, you should not have other commits from master in the PR. |
fa0df24 to
847e936
Compare
|
@zhoubecky is a new contributor, so lets get this reviewed soonish so they don't have to rebase again! |
|
@jklymak, the test_axes_grid1.py was not conflict yesterday, should I resolve the conflict in this branch or you will do the rebase? |
|
@zhoubecky can you do the rebase? |
|
@zhoubecky much easier for you to do it. If you haven't done a rebase before, make sure you make a backup branch so you can revert if you make a mistake. There are instructions in the developers guide. |
847e936 to
8e15b70
Compare
| def test_fill_facecolor(): | ||
| fig, ax = plt.subplots(1, 5) | ||
| fig.set_size_inches(5, 5) | ||
| trans1 = blended_transform_factory(ax[0].transData, ax[0].transData) |
There was a problem hiding this comment.
I don't understand this; blended_transform_factory is for separate x and y transforms, but you are using the same transform for both, so why not use ax[0].transData instead of trans1 (and the same for the rest)?
| axins = zoomed_inset_axes(ax[0], 1, loc=1) | ||
| axins.set_xlim(0, 0.2) | ||
| axins.set_ylim(0, 0.2) | ||
| plt.gca().axes.get_xaxis().set_ticks([]) |
There was a problem hiding this comment.
Why do you need to do plt.gca()? Don't you have all the Axes objects already?
|
Thank you @QuLogic , I have updated the test, is there anything I should do to make it better? |
|
Thanks a lot @zhoubecky ! |
PR Summary
fix BboxConnectorPatch does not show facecolor #8059 based on old fixes in #8075
will add command and unit tests later