Skip to content

fix y axis on 3d plot - #21424

Closed
janash wants to merge 1 commit into
matplotlib:mainfrom
janash:3dplot
Closed

janash wants to merge 1 commit into
matplotlib:mainfrom
janash:3dplot

Conversation

@janash

@janash janash commented Oct 21, 2021

Copy link
Copy Markdown

This commit corrects the axis which the 3D y axis inherits from. The y-axis on 3D plots initially inherited from the x axis object, rather than a y axis object. This resulted in a bug where the x axis (instead of the y) was inverted for 3D plots when was called. After inheritance correction, correct behavior was achieved with inversion, but the tick marks were incorrectly placed. The default location for tick marks on the y-axis is now
set to be right, rather than left.

Resolves #21369

PR Summary

See commit message. This PR unfortunately causes failure of most tests in mplot3D at the moment.

PR Checklist

  • correct axis inheritance
  • change y axis tick position
  • Determine if corrections cause problems elsewhere in the code base
  • Correct unit tests if necessary
  • Add unit test to check for proper axis inversion.
  • Has pytest style unit tests (and pytest passes).
  • Is Flake 8 compliant (run flake8 on changed files to check).
  • New features are documented, with examples if plot related.
  • Documentation is sphinx and numpydoc compliant (the docs should build without error).
  • Conforms to Matplotlib style conventions (install flake8-docstrings and run flake8 --docstring-convention=all).
  • New features have an entry in doc/users/next_whats_new/ (follow instructions in README.rst there).
  • API changes documented in doc/api/next_api_changes/ (follow instructions in README.rst there).

This commit corrects the axis which the 3D y axis inherits from. The y-axis on 3D plots initially inherited from the x axis object, rather than a y axis object. This resulted in a bug where the x axis (instead of the y) was inverted for 3D plots when  was called. After inheritance correction, correct behavior was achieved with inversion, but the tick marks were incorrectly placed. The default location for tick marks on the y-axis is now
set to be right, rather than left.

Resolves matplotlib#21369

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for opening your first PR into Matplotlib!

If you have not heard from us in a while, please feel free to ping @matplotlib/developers or anyone who has commented on the PR. Most of our reviewers are volunteers and sometimes things fall through the cracks.

You can also join us on gitter for real-time discussion.

For details on testing, writing docs, and our review process, please see the developer guide

We strive to be a welcoming and open project. Please follow our Code of Conduct.

@QuLogic

QuLogic commented Oct 21, 2021

Copy link
Copy Markdown
Member

Does this work okay if you rotate the axes so that the y axis is on the left side? There may be some automated alignment picker that assumes the labels were on the 'bottom of the x axis'.

I think the other test failures are in the wrong grid lines. I'll mark this Draft for now.

@QuLogic
QuLogic marked this pull request as draft October 21, 2021 23:19
@QuLogic QuLogic added this to the v3.6.0 milestone Oct 21, 2021
@janash

janash commented Oct 22, 2021

Copy link
Copy Markdown
Author

Good point. I will check for that.

@janash

janash commented Oct 22, 2021 •

Copy link
Copy Markdown
Author

I've tried rotation with both the feature and main versions of this code reported in the issue with the following results (attached). The first figure is from this feature branch, the second is from main branch.

feature_with_ylabel_rotated_200
main_with_ylabel_rotated_200

There is no resulting error, and with a rotation of 200 degrees, each appears okay. Is there a recommended more systematic way to test this?

There does seem to be something that needs to be fixed for this branch when a Y label is present (y label and tick labels overlap), so this is a to-do:

feature_with_ylabel

@QuLogic

QuLogic commented Oct 26, 2021

Copy link
Copy Markdown
Member

OK, that's good.

If you want to test directly at some direction, you can use Axes3D.view_init to set the view angles.

@janash

janash commented Oct 30, 2021 •

Copy link
Copy Markdown
Author

Hi @QuLogic - thanks. I did use view_init to set the angles for the images in the previous comment. To me, the labels seem okay. Should I attempt to fix the failing tests?

It seems like some failures may have to do with reference figures, and I'm not sure how I would approach this.

@janash

janash commented Nov 4, 2021

Copy link
Copy Markdown
Author

Image diffs for failing mplot3d tests look mostly like this. Would the proper course of action be to change the reference images?

image

@QuLogic

QuLogic commented Nov 5, 2021

Copy link
Copy Markdown
Member

That one is likely okay, but the ones with extra tick lines (e.g., axes3d_isometric.png) still needs investigating.

@tacaswell

Copy link
Copy Markdown
Member

I'm going to close this because we resolved the issue in a different way.

Thank you for your work @janash and I hope we hear from you again!

@tacaswell tacaswell closed this Dec 16, 2022
@tacaswell tacaswell removed this from the v3.7.0 milestone Dec 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: ax.invert_xaxis() and ax.invert_yaxis() both flip the X axis

3 participants