Skip to content

Add auto-scaling support to "add_collection3d" - #25751

Closed
lganic wants to merge 7 commits into
matplotlib:mainfrom
lganic:main
Closed

lganic wants to merge 7 commits into
matplotlib:mainfrom
lganic:main

Conversation

@lganic

@lganic lganic commented Apr 22, 2023 •

Copy link
Copy Markdown
Contributor

PR Summary

closes #17130

Adds support for auto-scaling 3D collection objects in the add_collection3d function. The new implementation is consistent with the behavior of the existing 2D implementation, where auto-scaling is True by default.

Test cases were added for each of the relevant autoscaling functions.

The autoscaling works with most 2D/3D collections; the only collection I wasn't able to get working was Patch3DCollection, which (as far as I can tell) seems to have a number of ongoing bugs that make them borderline unusable, and untestable.

Also turns out that a few of the test cases suffered from the bug seen in #17130, so I updated the failing tests accordingly.

PR Checklist

Linked Issue

  • Added "closes #0000" in the PR description to link it to the original issue.

Documentation and Tests

  • Has pytest style unit tests (and pytest passes)
  • Documentation is sphinx and numpydoc compliant (the docs should build without error).
  • New plotting related features are documented with examples.

Release Notes

  • New features are marked with a .. versionadded:: directive in the docstring and documented in doc/users/next_whats_new/
  • API changes are marked with a .. versionchanged:: directive in the docstring and documented in doc/api/next_api_changes/
  • Release notes conform with instructions in next_whats_new/README.rst or next_api_changes/README.rst

@scottshambaugh

scottshambaugh commented May 5, 2023 •

Copy link
Copy Markdown
Contributor

I believe this may also close #23317, and we should check if it closes #14298 as well.

@QuLogic QuLogic left a comment

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.

We would prefer if you could rebase instead of merge. It appears there are some completely unrelated commits in this PR that shouldn't be here.

# If a 2D collection was passed, it has been converted to 3D

if autolim:
if type(col) is art3d.Line3DCollection:

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.

Is there a reason to not use isinstance here?

@scottshambaugh

Copy link
Copy Markdown
Contributor

Hi @lganic, are you still interested in working on this PR?

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.

autoscale_view is not working with Line3DCollection

3 participants