8000 Add missing super __init__ in subclasses by QuLogic · Pull Request #20436 · matplotlib/matplotlib · GitHub
[go: up one dir, main page]

Skip to content

Add missing super __init__ in subclasses #20436

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

Merged
merged 4 commits into from
Jun 16, 2021

Conversation

QuLogic
Copy link
Member
@QuLogic QuLogic commented Jun 15, 2021

PR Summary

These can cause issues for some static checks (I noticed them in LGTM), and mostly the code in the super __init__ is duplicated in the subclass.

PR Checklist

  • Has pytest style unit tests (and pytest passes).
  • Is Flake 8 compliant (run flake8 on changed files to check).
  • [n/a] New features are documented, with examples if plot related.
  • [n/a] Documentation is sphinx and numpydoc compliant (the docs should build without error).
  • Conforms to Matplotlib style conventions (install flake8-docstrings and run flake8 --docstring-convention=all).
  • [n/a] New features have an entry in doc/users/next_whats_new/ (follow instructions in README.rst there).
  • [n/a] API changes documented in doc/api/next_api_changes/ (follow instructions in README.rst there).

QuLogic added 3 commits June 14, 2021 20:37
The ones created by `_make_norm_from_scale` all call it.
These probably worked fine due to using `set_canvas_size` before
`get_results`, or something that would set the remaining attributes.
The `vertices` and `codes` properties are overridden, so cannot be set
directly, but the arrays that back them can be modified. This seems
unintentional, so disable writing on those as well.
@@ -385,11 +385,11 @@ def __init__(self, xy, s, size=None, prop=None,

self._cached_vertices = None
s, ismath = Text(usetex=usetex)._preprocess_math(s)
self._vertices, self._codes = text_to_path.get_text_path(
prop, s, ismath=ismath)
super().__init__(
Copy link
Contributor

Choose a reason for hiding this comment

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

perhaps rather use fast_from_codes_and_verts for speed?

Copy link
Member Author

Choose a reason for hiding this comment

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

That's a class method / constructor, though?

Copy link
Contributor

Choose a reason for hiding this comment

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

ah yes. Oh well, I guess this isn't a bottleneck...

@@ -189,6 +189,7 @@ class MathtextBackendPs(MathtextBackend):
"_PSResult", "width height depth pswriter used_characters")

def __init__(self):
super().__init__()
Copy link
Member

Choose a reason for hiding this comment

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

Are none of these really ever tested? Maybe beyond this PR, but they should be if they aren't...

Copy link
Member Author

Choose a reason for hiding this comment

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

They're almost all unused, which I guess is why they're deprecated. For the ones that are used, I assume they all get .set_canvas_size called first, which sets the things that were missed in the super __init__.

@tacaswell tacaswell merged commit 6939574 into matplotlib:master Jun 16, 2021
@tacaswell tacaswell added this to the v3.5.0 milestone Jun 16, 2021
@QuLogic QuLogic deleted the super-init branch June 16, 2021 19:35
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.

4 participants
0