Skip to content

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

Closed
patquem wants to merge 0 commit into
matplotlib:mainfrom
patquem:main
Closed

[ENH] : Provide axis('equal') for Axes3D #22570#22705
patquem wants to merge 0 commit into
matplotlib:mainfrom
patquem:main

Conversation

@patquem

@patquem patquem commented Mar 25, 2022

Copy link
Copy Markdown
Contributor

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

@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.

@oscargus

oscargus commented Mar 25, 2022

Copy link
Copy Markdown
Member

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

patquem commented Mar 29, 2022

Copy link
Copy Markdown
Contributor Author

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

oscargus commented Apr 1, 2022

Copy link
Copy Markdown
Member

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

patquem commented Apr 1, 2022

Copy link
Copy Markdown
Contributor Author

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.

Comment thread lib/mpl_toolkits/mplot3d/axes3d.py Outdated
value description
========= ==================================================
'auto' automatic; fill the position rectangle with data.
'equal' automatic; adapt the axes to have equal aspect

Copy link
Copy Markdown
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.

Comment thread lib/mpl_toolkits/mplot3d/axes3d.py Outdated
mpl_toolkits.mplot3d.axes3d.Axes3D.set_box_aspect
"""
if aspect != 'auto':
if aspect not in ['auto', 'equal']:

Copy link
Copy Markdown
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

oscargus commented Apr 5, 2022

Copy link
Copy Markdown
Member

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

@oscargus

oscargus commented Apr 5, 2022

Copy link
Copy Markdown
Member

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

oscargus commented Apr 5, 2022

Copy link
Copy Markdown
Member

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

oscargus commented Apr 5, 2022

Copy link
Copy Markdown
Member

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

patquem commented Apr 5, 2022

Copy link
Copy Markdown
Contributor Author

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

oscargus commented Apr 7, 2022

Copy link
Copy Markdown
Member

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