FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

HostAxesBase now adds appropriate _remove_method to its parasite axes. by smheidrich · Pull Request #4898 · matplotlib/matplotlib · GitHub

  1. matplotlib
  2. matplotlib

Repository navigation

HostAxesBase now adds appropriate _remove_method to its parasite axes. - #4898

Merged
mdboom merged 5 commits into
matplotlib:masterfrom
smheidrich:add_remove_to_axes_grid1_twin_axes
Oct 15, 2015
ConversationCommits (5)ChecksFiles changed
Merged

HostAxesBase now adds appropriate _remove_method to its parasite axes.#4898
mdboom merged 5 commits into
matplotlib:masterfrom
smheidrich:add_remove_to_axes_grid1_twin_axes

Conversation

smheidrich commented Aug 10, 2015

Copy link
Copy Markdown
Contributor

Fix for #4896 .
I think this should do the trick.

tacaswell commented Aug 11, 2015

Copy link
Copy Markdown
Member

Can you also add tests and a what_new entry?

tacaswell added this to the proposed next point release milestone Aug 11, 2015

smheidrich commented Aug 11, 2015

Copy link
Copy Markdown
Contributor Author

Hmm there seems to be a problem with removing twinx and twiny axes where the lines and ticks remain, only the numbers disappear. Will look into this later.

smheidrich commented Aug 11, 2015

Copy link
Copy Markdown
Contributor Author

Ok it's because the twin* methods also screw with the host axes' visibility settings. I don't know if and how this should be fixed.

smheidrich commented Aug 11, 2015

Copy link
Copy Markdown
Contributor Author

Are you sure you want a whats_new entry for this? Seems like more of a minor bug fix than something worth mentioning there...

tacaswell commented Aug 12, 2015

Copy link
Copy Markdown
Member

I like to advertise when we add long standing missing functionality.

It also serves as advertisement that the remove functionality exists on all
of the other artists.

If you don't want to do it i will leave it to your judgement.

On Tue, Aug 11, 2015, 7:01 PM productivememberofsociety666 <
notifications@github.com> wrote:

Are you sure you want a whats_new entry for this? Seems like more of a
minor bug fix than something worth mentioning there...

—
Reply to this email directly or view it on GitHub
#4898 (comment)
.

tacaswell commented Aug 12, 2015

Copy link
Copy Markdown
Member

Could you reduce the tests to only having pngs ?

smheidrich commented Aug 12, 2015

Copy link
Copy Markdown
Contributor Author

I'm trying to understand what the point is behind the host axes' visibility settings being changed by the twin* functions. Here is what it looks like if I just set them all to invisible:

So the removal code works at least, but the twin* functions seem a bit inconsistent: twinx and twiny set their lines invisible and show only the ticks and numbers, whereas twin has everything visible.

smheidrich commented Aug 12, 2015

Copy link
Copy Markdown
Contributor Author

Regarding the failing build, I don't know why the pictures aren't close. It works on my machine with all Python versions from 2.6 to 3.4. Fonts maybe?

tacaswell commented Aug 12, 2015

Copy link
Copy Markdown
Member

Make sure that you don't have any funny rcparams set (which should not
matter as the tests should be overriding that).

Also check what version of freetype you have installed against what is on
travis.

On Wed, Aug 12, 2015 at 6:33 PM productivememberofsociety666 <
notifications@github.com> wrote:

Regarding the failing build, I don't know why the pictures aren't close.
It works on my machine with all Python versions from 2.6 to 3.4. Fonts
maybe?

—
Reply to this email directly or view it on GitHub
#4898 (comment)
.

tacaswell commented Aug 12, 2015

Copy link
Copy Markdown
Member

Can you also squash this down to remove the un-used images from the history? Part of the goal is to keep the size of the repository down.

smheidrich force-pushed the add_remove_to_axes_grid1_twin_axes branch 2 times, most recently from fc5241b to 1910487 Compare August 12, 2015 23:32

smheidrich commented Aug 12, 2015

Copy link
Copy Markdown
Contributor Author

All right I took out an rcparam that set the backend to GTK3 and uploaded a slightly modified image. Now apparently all builds work except that 2.7 one due to another test failing...

QuLogic commented Aug 13, 2015

Copy link
Copy Markdown
Member

Your first few commits were made without setting your author info; you should update them for reasonable attribution.

smheidrich force-pushed the add_remove_to_axes_grid1_twin_axes branch from 1910487 to 9c24d49 Compare August 13, 2015 10:05

smheidrich commented Aug 13, 2015

Copy link
Copy Markdown
Contributor Author

Build passes now although I didn't change anything. The test that failed and then magically worked was mathfont_stix_53 in test_mathext, see https://travis-ci.org/matplotlib/matplotlib/jobs/75359652.

smheidrich force-pushed the add_remove_to_axes_grid1_twin_axes branch from 9c24d49 to e89181d Compare August 13, 2015 10:39

tacaswell commented Aug 13, 2015

Copy link
Copy Markdown
Member

There is a race condition in the math text rendering tests that
intermittently fails.

On Thu, Aug 13, 2015, 6:35 AM productivememberofsociety666 <
notifications@github.com> wrote:

Build passes now although I didn't change anything. The test that failed
and then magically worked was mathfont_stix_53 in test_mathext, see
https://travis-ci.org/matplotlib/matplotlib/jobs/75359652.

—
Reply to this email directly or view it on GitHub
#4898 (comment)
.

tacaswell commented Aug 14, 2015

Copy link
Copy Markdown
Member

I think the reason that the twin* functions mess with the visibility is to prevent any artifacts from rendering nominally identical artists (the ticks) on top of each other.

I think that this remove behavior should also restore the visibility settings of the host axes.

Can you also check if the twin functions mutate any other data structures in the axes (I know the normal axes.Axes.twin* methods do)? That should probably be un-done as well.

smheidrich commented Aug 14, 2015

Copy link
Copy Markdown
Contributor Author

I extended the tests to show better what is going on. Here is what things look like with the current twin* functionality:

Like I already said, there is inconsistency between the twinx/twiny methods and the bare twin method (axis line visibility).

In my opinion, it would be better if all methods behaved like twin, i.e. make corresponding host axes completely invisible. The last commit contains code that does just that and also makes the _remove_method reverse these changes. Here is what things look like then:

As for your question, I haven't seen any other changes being made to the host axes by the twin* functions.

tacaswell commented Aug 14, 2015

Copy link
Copy Markdown
Member

Awesome 👍

Thanks for wading into this!

I am not going to hold 1.5 for this PR, but have no problem with it sneaking in (it is far less disruptive than other open PRs).

smheidrich force-pushed the add_remove_to_axes_grid1_twin_axes branch from c8f67b6 to 51d49be Compare August 14, 2015 18:03

mdboom commented Oct 6, 2015

Copy link
Copy Markdown
Member

Looks like a great bugfix. Looks like this needs a rebase, then another run through Travis, but then I'm happy to merge.

smheidrich force-pushed the add_remove_to_axes_grid1_twin_axes branch from 51d49be to 0183a67 Compare October 8, 2015 20:16
smheidrich force-pushed the add_remove_to_axes_grid1_twin_axes branch from 0183a67 to 0fa9e74 Compare October 8, 2015 20:26
smheidrich force-pushed the add_remove_to_axes_grid1_twin_axes branch 4 times, most recently from 9146ab3 to 4760c14 Compare October 15, 2015 15:00
smheidrich force-pushed the add_remove_to_axes_grid1_twin_axes branch from 4760c14 to d268abf Compare October 15, 2015 15:01

smheidrich commented Oct 15, 2015

Copy link
Copy Markdown
Contributor Author

Rebase done!

mdboom added a commit that referenced this pull request Oct 15, 2015
Merge pull request #4898 from productivememberofsociety666/add_remove…
e7e8bcb 
…_to_axes_grid1_twin_axes

HostAxesBase now adds appropriate _remove_method to its parasite axes.
mdboom merged commit e7e8bcb into matplotlib:master Oct 15, 2015
afvincent mentioned this pull request Jul 27, 2016
Closed
smheidrich mentioned this pull request Jul 31, 2016
Closed
smheidrich mentioned this pull request Oct 2, 2018
Closed
smheidrich mentioned this pull request Jan 3, 2019
Draft
3 of 7 tasks
smheidrich added a commit to smheidrich/matplotlib that referenced this pull request Jan 3, 2019
axes_grid1: make twin* not modify host axes
a54f4c7 
PR matplotlib#4898 changed the behavior of twinx() and twiny() in axes_grid1 to
make them consistent with twin(). This commit inverts the "direction" of
consistency and makes twin() behave like the pre-matplotlib#4898 twinx() and
twiny() instead. This way, the API becomes less surprising: instead of
hiding certain host axes, twin* now keeps the host axes unmodified,
while the parasite axes have their axis lines hidden instead.

Unfortunately, this change breaks backwards-compatibility for code
relying on the specifics of host and parasite axes visibility.

Helps with matplotlib#10748.
smheidrich added a commit to smheidrich/matplotlib that referenced this pull request Jan 3, 2019
axes_grid1: make twin* not modify host axes
2d25e62 
PR matplotlib#4898 changed the behavior of twinx() and twiny() in axes_grid1 to
make them consistent with twin(). This commit inverts the "direction" of
consistency and makes twin() behave like the pre-matplotlib#4898 twinx() and
twiny() instead. This way, the API becomes less surprising: instead of
hiding certain host axes, twin* now keeps the host axes unmodified,
while the parasite axes have their axis lines hidden instead.

Unfortunately, this change breaks backwards-compatibility for code
relying on the specifics of host and parasite axes visibility.

Helps with matplotlib#10748.
smheidrich added a commit to smheidrich/matplotlib that referenced this pull request Jan 3, 2019
axes_grid1: make twin* not modify host axes
e499b2f 
PR matplotlib#4898 changed the behavior of twinx() and twiny() in axes_grid1 to
make them consistent with twin(). This commit inverts the "direction" of
consistency and makes twin() behave like the pre-matplotlib#4898 twinx() and
twiny() instead. This way, the API becomes less surprising: instead of
hiding certain host axes, twin* now keeps the host axes unmodified,
while the parasite axes have their axis lines hidden instead.

Unfortunately, this change breaks backwards-compatibility for code
relying on the specifics of host and parasite axes visibility.

Helps with matplotlib#10748.
smheidrich added a commit to smheidrich/matplotlib that referenced this pull request Jan 4, 2019
axes_grid1: make twin* not modify host axes
23a3b6e 
PR matplotlib#4898 changed the behavior of twinx() and twiny() in axes_grid1 to
make them consistent with twin(). This commit inverts the "direction" of
consistency and makes twin() behave like the pre-matplotlib#4898 twinx() and
twiny() instead. This way, the API becomes less surprising: instead of
hiding certain host axes, twin* now keeps the host axes unmodified,
while the parasite axes have their axis lines hidden instead.

Unfortunately, this change breaks backwards-compatibility for code
relying on the specifics of host and parasite axes visibility.

Helps with matplotlib#10748.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

v2.1

Development

Successfully merging this pull request may close these issues.

4 participants

Back | FazBrowse Home | New Git URL