8000 [Doc]: Better document Axes.transData and other transXYZ attributes · Issue #25220 · matplotlib/matplotlib · GitHub
[go: up one dir, main page]

Skip to content

[Doc]: Better document Axes.transData and other transXYZ attributes #25220

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

Open
timhoffm opened this issue Feb 15, 2023 · 10 comments · May be fixed by #25922
Open

[Doc]: Better document Axes.transData and other transXYZ attributes #25220

timhoffm opened this issue Feb 15, 2023 · 10 comments · May be fixed by #25922

Comments

@timhoffm
Copy link
Member

Documentation Link

No response

Problem

It's documented in the tutorial https://matplotlib.org/stable/tutorials/advanced/transforms_tutorial.html#data-coordinates, but not very discoverable:

Problem: transData and the likes are only attributes set in a [private function]/(

self.transData = self.transScale + (self.transLimits + self.transAxes)
)
As such, they do not have a docstring and do not appear in the API documentation.

Suggested improvement

Action: Find out, how to make this available in the API docs at https://github.com/matplotlib/matplotlib/blob/main/doc/api/axes_api.rst

Worth checking:

Alternative: Turn the trans* attributes into properties. They can be documented properly and have the advantage of being read-only. This is a slight performance cost, but should be negligible compared to any action involving the transforms.

@tacaswell
Copy link
Member

I think using a pattern like in #25187 to document class attributes (defaulting to None) might work as well?

@timhoffm
Copy link
Member Author

That pattern would work but feels a bit contrived. The class attributes are None and exist only for the purpose of documentation. Upon instance initialization they are immediately shadowed by instance attributes.

I'm leaning towards turning these into properties, because they are read-only.

@mwaskom
Copy link
mwaskom commented Feb 17, 2023

Better docs for the transforms would be great but fwiw I’m fairly certain the insights page counts all searches and then labels them with the most recent search, rather than showing the count for a specific query.

@AnanasClassic
Copy link

I would like to take up this issue

@ksunden
Copy link
Member
ksunden commented May 19, 2023

Properties probably make sense, especially if we want setting to be private behavior...

In the alternative, I do think that if we want to go the "class attribute" route, I'm not super happy with = None, as that introduces a different type that is not actually valid for any instance. Pep 526 allows a pure declaration (without initialization, but you have to put the type hint in... This is slightly odd in a setting where the actual type hints are in a parallel file, but it should work:

class AxesBase(...):
    ...
    transData: Transform
    """Docstring"""
    ...

This is a declaration that transData will exist and have type Transform, but I don't have an initial value yet (it will still actually give AttributeError if accessed before it is initialized)
I do think that all of the transXYZ transforms from that method should be documented, not only transData, as well.

@AnanasClassic
Copy link

maybe it is acceptable to initialize transData some default Transform-object ?

@AnanasClassic
Copy link

I think in order to document all the attributes, we need to create a new issue

@tacaswell tacaswell linked a pull request May 19, 2023 that will close this issue
5 tasks
@tacaswell tacaswell changed the title [Doc]: Better document Axes.transData [Doc]: Better document Axes.transData and other transXYZ attributes May 19, 2023
@tacaswell
Copy link
Member

I have updated this issue to cover all the transfrom attributes.

@AnanasClassic
Copy link

Do you have any ideas how to document this?
I think I can write something like

@property
def get_trans_data(self):
    """docstring"""
    return self.transData

But I'm afraid that in this case the clarity of the documentation will suffer

Or I can initialize transXYZ with default values (I don't quite understand what values to initialize them with)

@timhoffm
Copy link
Member Author

You should make the current attribute private self._transData and use the original name as a read-only accessor

@property
def transData(self):
    “””docstring”””
    return self._transData

AnanasClassic added a commit to AnanasClassic/matplotlib that referenced this issue May 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants
0