Skip to content

[ENH] : Provide axis('equal') for Axes3D #22570 #22705

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.

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

Closed
wants to merge 0 commits into from

Conversation

patquem
Copy link
Contributor

@patquem patquem commented Mar 25, 2022

PR Summary

Closes #22570
Make axis('equal') available
(I am new in PR - sorry for not doing things correctly)

PR Checklist

Tests and Styling

  • Has pytest style unit tests (and pytest passes).
  • Is Flake 8 compliant (install flake8-docstrings and run flake8 --docstring-convention=all).

Documentation

  • New features are documented, with examples if plot related.
  • New features have an entry in doc/users/next_whats_new/ (follow instructions in README.rst there).
  • [N/A] API changes documented in doc/api/next_api_changes/ (follow instructions in README.rst there).
  • Documentation is sphinx and numpydoc compliant (the docs should build without error).

Example

import numpy as np
import matplotlib.pyplot as plt


def plot_hexahedron(ax, x, y, z, *args, **kwargs):
    def plot_edge(i, j):
        ax.plot([x[i], x[j]], [y[i], y[j]], [z[i], z[j]], *args, **kwargs)

    # edges associated to the y=0 face
    plot_edge(0, 1)
    plot_edge(1, 3)
    plot_edge(3, 2)
    plot_edge(2, 0)

    # edges associated to the y=1 face
    plot_edge(4, 5)
    plot_edge(5, 7)
    plot_edge(7, 6)
    plot_edge(6, 4)

    # missing edges (x=cste)
    plot_edge(0, 4)
    plot_edge(1, 5)
    plot_edge(3, 7)
    plot_edge(2, 6)


def ex_hexahedron():
    """ Example of hexahedron representation with Matplotlib """
    points = np.array([[0, 0, 0], [1, 0, 0], [0, 1, 0], [1, 1, 0],
                       [0, 0, 3], [1, 0, 3], [0, 1, 3], [1, 1, 3]])

    x = np.asarray([coord[0] for coord in points])
    y = np.asarray([coord[1] for coord in points])
    z = np.asarray([coord[2] for coord in points])

    fig, axes = plt.subplots(1, 2, figsize=(12, 5),
                             subplot_kw={'projection': '3d'})

    def plot(ax):
        plot_hexahedron(ax, x, y, z, 'r')
        ax.set_xlabel("X")
        ax.set_ylabel("Y")
        ax.set_zlabel("Z")

    # standard representation (aspect='auto')
    plot(axes[0])
    axes[0].set_title("Standard representation")

    # 3d axis equal (aspect='equal')
    plot(axes[1])
    axes[1].set_aspect('equal')
    axes[1].set_title("set_aspect('equal)")
    plt.tight_layout()


ex_hexahedron()
plt.show()

image

Copy link

@github-actions github-actions bot left a comment

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.

@oscargus
Copy link
Member

oscargus commented Mar 25, 2022

Thanks!

The failure comes from a test that tests that equal is not supported...

def test_aspect_equal_error():
fig = plt.figure()
ax = fig.add_subplot(projection='3d')
with pytest.raises(NotImplementedError):
ax.set_aspect('equal')

So this should be replaced with some test testing the outcome of setting aspect='equal'. For example, something similar to the one that you have.

For the actual implementation, I cannot really say much as I do not know the part of this code very well. One can note that it partly seems to work, but that it may also need some adjustments. For example, the 2.0 value of the x-axis is cropped and it looks like the hexahedron is from 0.5 to 1.5 in y-direction.

@patquem
Copy link
Contributor Author

patquem commented Mar 29, 2022

Hi @oscargus,
I have changed the test.
I agree with you concerning the visualization (confusing in the Y-direction) but I have no idea of how to fix that.
Patrick

@oscargus
Copy link
Member

oscargus commented Apr 1, 2022

It seems like it is the location of the ytick-labels that is off. The -1.0 label is outside of the viewable area and all other are shifted one position. So I guess that the actual box is drawn correctly, but the labels are misplaced.

Edit: To clarify, I guess that this problem is not caused by your change, so it seems to be an issue elsewhere.

@patquem
Copy link
Contributor Author

patquem commented Apr 1, 2022

I agree with you.

Concerning the test, I will try to finalize it next week.
Presently, the figure I gave for comparison was generated with my current working env.
This engenders some differences with figure generated by the main branch.
I have to generate a new figure from my forked (and updated) matplotlib branch, to be coherent with it, I guess.

Possible values:

========= ==================================================
value description
========= ==================================================
'auto' automatic; fill the position rectangle with data.
'equal' automatic; adapt the axes to have equal aspect
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
'equal' automatic; adapt the axes to have equal aspect
'equal' automatic; adapt the axes to have equal aspect.

@@ -322,14 +316,27 @@ def set_aspect(self, aspect, adjustable=None, anchor=None, share=False):
--------
mpl_toolkits.mplot3d.axes3d.Axes3D.set_box_aspect
"""
if aspect != 'auto':
if aspect not in ['auto', 'equal']:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if aspect not in ['auto', 'equal']:
if aspect not in ('auto', 'equal'):

Minor thing, but slightly faster this way as this can be "compiled".

@oscargus
Copy link
Member

oscargus commented Apr 5, 2022

Approved subject to minor fixes and rebasing/squashing into single commit.

@oscargus
Copy link
Member

oscargus commented Apr 5, 2022

The error comes from adding a figure in one commit and updating in another. If you rebase and squash the commits into a single one, it should go away.

@oscargus
Copy link
Member

oscargus commented Apr 5, 2022

Oh, you are using the main branch. In general it is better to not do that, but to create a separate branch.

Now, you need to do something like
git rebase -i HEAD~4
(HEAD minus four commits)
change pick to fixup for the last three.
git push --force-with-lease

Then, for later updates you can do

git commit --amend

so that the change will stay in that single commit.

@oscargus
Copy link
Member

oscargus commented Apr 5, 2022

We can also squash the commits when merging.

@oscargus oscargus added the status: needs workflow approval For PRs from new contributors, from which GitHub blocks workflows by default. label Apr 5, 2022
@patquem
Copy link
Contributor Author

patquem commented Apr 5, 2022

Hi @oscargus ,
It sounds like chinese for me.
I have tried to do what you wrote. Not sure I did the things correctly. (?)
image

@oscargus
Copy link
Member

oscargus commented Apr 7, 2022

I think that may not have worked, but do not worry, we can sort it out while merging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: duplicate status: needs workflow approval For PRs from new contributors, from which GitHub blocks workflows by default. topic: mplot3d
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ENH]: Provide axis('equal') for Axes3D.
3 participants