Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Improve mpl_toolkit documentation#23536

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
QuLogic merged 1 commit intomatplotlib:mainfromoscargus:mpltoolkitdocs
Oct 21, 2022

Conversation

oscargus
Copy link
Member

@oscargusoscargus commentedAug 1, 2022
edited
Loading

PR Summary

A bit of links and numpydocifying.

(Probably won't work at the first attempt as I cannot build locally...)

And a minor change in the code for efficiency.

PR Checklist

Tests and Styling

  • [N/A] Has pytest style unit tests (andpytest passes).
  • IsFlake 8 compliant (installflake8-docstrings and runflake8 --docstring-convention=all).

Documentation

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

@oscargusoscargusforce-pushed thempltoolkitdocs branch 6 times, most recently from0f066c3 tocc71611CompareAugust 2, 2022 10:21
@oscargusoscargus marked this pull request as ready for reviewAugust 2, 2022 10:21
Copy link
Member

@QuLogicQuLogic left a comment

Choose a reason for hiding this comment

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

BTW, haven't commented everywhere, but I think there's no need to do~.ClassName, as the~ trims the displayed nameas specified, so there's nothing to trim. It'd only have an effect if you did, e.g.,~.module.ClassName.

@oscargusoscargus marked this pull request as draftAugust 8, 2022 22:23
@oscargusoscargusforce-pushed thempltoolkitdocs branch 4 times, most recently frome88ef90 to14b3aafCompareAugust 17, 2022 09:15
@oscargusoscargus marked this pull request as ready for reviewAugust 17, 2022 09:17
@oscargus
Copy link
MemberAuthor

I am a bit confused if one should keep the documentation formpl_toolkit "separated" frommatplotlib, i.e., if one should use full descriptorsmatplotlib.axes.Axes to show that it is amatplotlib class or~.axes.Axes for brevity...

@timhoffm
Copy link
Member

I don't think we need extra separation when referencing types inmpl_toolkits. Sohttps://matplotlib.org/stable/devel/documenting_mpl.html#referencing-types still applies.

@oscargus
Copy link
MemberAuthor

In#19572 you,@timhoffm , suggested to makeset_3d_properties private. Is that still your standpoint?

There's also#1482 and partly#1483 which may be relevant to consider.

@timhoffm
Copy link
Member

In#19572 you,@timhoffm , suggested to makeset_3d_properties private. Is that still your standpoint?

Yes.

  • It should be an implementation detail that our 3D artists are 2D artists with a third dimension glued on.set_3d_properties leaks this concept. Having this public would make it much more difficult to replace Artists later with proper 3D Artists, in case we see the need for it.
  • The API is confusing: I can update x and y with one function, but have to use another function for z. That's weird. - Problem: We do not have actual 3D data setters in many cases.set_3d_properties is currently the only way. Before we can privatize it, we need a proper 3D set_data API.
  • The API is inconsistent between different Artists. Migrating this API to a homogeneousset_3d_properties interface is likely at least as much work as deprecating and providing set_data.

@oscargusoscargusforce-pushed thempltoolkitdocs branch 4 times, most recently from342d48d tob95b4abCompareOctober 15, 2022 22:01
Copy link
Member

@timhoffmtimhoffm left a comment

Choose a reason for hiding this comment

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

There are code cleanups in here, which makes this require 2 approvals.

@oscargusoscargusforce-pushed thempltoolkitdocs branch 2 times, most recently from6c09ea0 to1b4f2f7CompareOctober 21, 2022 10:16
@QuLogicQuLogic merged commitc450aa7 intomatplotlib:mainOct 21, 2022
@oscargusoscargus deleted the mpltoolkitdocs branchOctober 22, 2022 05:47
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@QuLogicQuLogicQuLogic approved these changes

@timhoffmtimhoffmtimhoffm approved these changes

Assignees
No one assigned
Projects
None yet
Milestone
v3.7.0
Development

Successfully merging this pull request may close these issues.

3 participants
@oscargus@timhoffm@QuLogic

[8]ページ先頭

©2009-2025 Movatter.jp