Skip to content

Conversation

@oscarbates
Copy link
Contributor

Two changes made to facilitate jobs. Improvement to findomp and a 3D geometry plotter that doesn't depend on mayavi

return axis


def plot_points_3d(coordinates, axis=None, colour='red', size=15, title=None, **kwargs):
Copy link
Contributor

Choose a reason for hiding this comment

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

could you change the name to plot_points_3d_matplotlib?

ax.grid(True)
ax.set_box_aspect([1, 1, 0.5])
plt.tight_layout()
plt.show()
Copy link
Contributor

Choose a reason for hiding this comment

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

in all of these functions, we return the axis instead of plotting inside the function - remove these last two lines and change to return ax

title : str, optional
Figure title, defaults to empty title.
"""
fig = plt.figure(figsize=(10, 8))
Copy link
Contributor

Choose a reason for hiding this comment

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

check how other matplotlib functions are written, where we let the user provide some external figure/axes and, if not, we create create ones

Size of the plotted points, defaults to 15.
title : str, optional
Figure title, defaults to empty title.
"""
Copy link
Contributor

Choose a reason for hiding this comment

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

check other matplotlib functions to add display test at top

"""
fig = plt.figure(figsize=(10, 8))
ax = fig.add_subplot(111, projection='3d')
ax.set_title('3D View: Tangent Points with Out-of-Plane Displacement')
Copy link
Contributor

Choose a reason for hiding this comment

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

title should be provided externally as parameter, as done in other functions like this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants