Skip to content

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

New issue

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

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 5 commits into from
Oct 15, 2015
Merged

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

merged 5 commits into from
Oct 15, 2015

Conversation

smheidrich
Copy link
Contributor

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

@tacaswell
Copy link
Member

Can you also add tests and a what_new entry?

@tacaswell tacaswell added this to the proposed next point release milestone Aug 11, 2015
@smheidrich
Copy link
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
Copy link
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
Copy link
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
Copy link
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
Copy link
Member

Could you reduce the tests to only having pngs ?

@smheidrich
Copy link
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:
twin_axes_empty_and_removed
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
Copy link
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
Copy link
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
Copy link
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 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
Copy link
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
Copy link
Member

QuLogic commented Aug 13, 2015

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

@smheidrich smheidrich force-pushed the add_remove_to_axes_grid1_twin_axes branch from 1910487 to 9c24d49 Compare August 13, 2015 10:05
@smheidrich
Copy link
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 smheidrich force-pushed the add_remove_to_axes_grid1_twin_axes branch from 9c24d49 to e89181d Compare August 13, 2015 10:39
@tacaswell
Copy link
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
Copy link
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
Copy link
Contributor Author

I extended the tests to show better what is going on. Here is what things look like with the current twin* functionality:
old twin_axes_empty_and_removed
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:
twin_axes_empty_and_removed

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

@tacaswell
Copy link
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 smheidrich force-pushed the add_remove_to_axes_grid1_twin_axes branch from c8f67b6 to 51d49be Compare August 14, 2015 18:03
@mdboom
Copy link
Member

mdboom commented Oct 6, 2015

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

@smheidrich smheidrich force-pushed the add_remove_to_axes_grid1_twin_axes branch from 51d49be to 0183a67 Compare October 8, 2015 20:16
@smheidrich smheidrich force-pushed the add_remove_to_axes_grid1_twin_axes branch from 0183a67 to 0fa9e74 Compare October 8, 2015 20:26
@smheidrich 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 smheidrich force-pushed the add_remove_to_axes_grid1_twin_axes branch from 4760c14 to d268abf Compare October 15, 2015 15:01
@smheidrich
Copy link
Contributor Author

Rebase done!

mdboom added a commit that referenced this pull request Oct 15, 2015
@mdboom
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 mdboom merged commit e7e8bcb into matplotlib:master Oct 15, 2015
@afvincent afvincent mentioned this pull request Jul 27, 2016
Closed
@smheidrich smheidrich mentioned this pull request Jul 31, 2016
Closed
@smheidrich smheidrich mentioned this pull request Oct 2, 2018
Closed
@smheidrich smheidrich mentioned this pull request Jan 3, 2019
Draft
7 tasks
smheidrich added a commit to smheidrich/matplotlib that referenced this pull request Jan 3, 2019
@smheidrich
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
@smheidrich
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
@smheidrich
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
@smheidrich
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.
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
@smheidrich @tacaswell @QuLogic @mdboom