-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
axes3d panning #18536
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
axes3d panning #18536
Conversation
Can you write a What's new entry for this? You also have flake8 issues that would need to be fixed. |
@QuLogic sorry about that, I'm not very familiar with the proper formatting. I fixed the mentioned flake8 issues, but what's a |
For new features, we add a "what's new" entry that gets collated into a
single document for release. The instructions are here:
https://matplotlib.org/3.3.1/users/next_whats_new/README.html
…On Tue, Sep 22, 2020 at 6:16 AM Emily ***@***.***> wrote:
@QuLogic <https://github.com/QuLogic> sorry about that, I'm not very
familiar with the proper formatting. I fixed the mentioned flake8 issues,
but what's a what's new entry?
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#18536 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AACHF6FHX55SEXWLDEP74LTSHB2RPANCNFSM4RUCNAFQ>
.
|
Thanks for this @emilyfy ! I suspect the reason it was not implemented is no one had implemented it yet ;). I checked this out and verified that it works as expected. Given that this whole block of code is untested I am not going to block merging on that, but if you are feeling motivated, following the pattern in https://github.com/matplotlib/matplotlib/blob/master/lib/matplotlib/testing/widgets.py is the path to go. I agree that this is a big enough feature added that we need to make sure it gets advertised in the whats new! |
I added a short what's new entry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This works pretty well for me. I'm not sure we really have any good way to test 3D interactivity.
def do_event(tool, etype, button=1, xdata=0, ydata=0, key=None, step=1): | ||
""" | ||
Trigger an event | ||
def mock_event(ax, button=1, xdata=0, ydata=0, key=None, step=1): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Git diff looks more complicated than it is. This just extracts the event creation out of do_event()
.
@emilyfy Sorry, for being a bit confusing. Writing the test was actually a bit more difficult than thought, because the ping @tacaswell @QuLogic for re-review/merge. |
Thanks @emilyfy! Congratulations on your first contribution to Matplotlib. We'd be happy to see you back some time. |
PR Summary
not sure why axes3d panning with middle scroll button is not implemented. I'm putting it here and it works as far as I tested.
PR Checklist
pytest
passes).flake8
on changed files to check).flake8-docstrings
andpydocstyle<4
and runflake8 --docstring-convention=all
).doc/users/next_whats_new/
(follow instructions in README.rst there).doc/api/next_api_changes/
(follow instructions in README.rst there).