Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork8.2k
Better document Axes.transData and other transXYZ attributes#25922
Better document Axes.transData and other transXYZ attributes#25922AnanasClassic wants to merge 8 commits intomatplotlib:mainfrom
Conversation
jklymak commentedMay 19, 2023
Thanks for this. However, can you add a better title and move the cross reference to the description? |
Uh oh!
There was an error while loading.Please reload this page.
tacaswell commentedMay 19, 2023
tacaswell left a comment
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.
- I do not think that there is consensus on the best way to fix this per#25220 (comment)
- If we are going to do this for one of the
transXYZwe should to it for all of them in the same PR.
timhoffm commentedMay 22, 2023
@tacaswell let's come to a decision
I read the linked comment by@ksunden as +0.5 for properties. I'm also +0.5 on properties (and would make them read-only) I can't read any preference from your comments. |
AnanasClassic commentedMay 22, 2023
as I see it, we are going to properties. I think I implemented this in my last commit, and I'm ready to listen to what I should fix in my implementation. I also have a slightly naive question about failed tests: I don't quite understand how the changes I made caused errors in the tests (as far as I understand, the errors say about the conflict of Tkinter versions) |
timhoffm commentedMay 24, 2023
I see it the same way, but@tacaswell doesn't seem so sure. Let's wait for his reply. The test failures are unrelated. This is a configuration problem in the CI system for a specific version of OSX and tk. |
tacaswell commentedMay 25, 2023
I added this to today's call agenda. Moving to properties to get documentation seems backwards to me (we are imposing a run-time cost for a doc-build-time problem). These have not changed is years, could we explicitly list them in the class docstring instead? |
timhoffm commentedMay 25, 2023
I‘m not sure I will make it to the call. Therefore, I add my 2cents right here: Adding runtime cost just for doc would indeed sound odd, however
Overall I‘m +0.5 on properties. But it should alternatively be viable to list them in an |
tacaswell commentedMay 25, 2023
The PR currently has setters for all 4. |
tacaswell commentedMay 25, 2023
Consensus from call
|
rcomer commentedJul 15, 2023
Hi@AnanasClassic are you still interested in working on this one? |
Uh oh!
There was an error while loading.Please reload this page.
PR summary
Closes#25220
I added in _base.py defining the Axes.transData attribute and added documentation generation for it to axes_api.rst. It seems to me that this is enough for the user to understand from the github documentation what Axes.transData is.
PR checklist