Skip to content

Deprecate PackerBase #14526

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

Closed
wants to merge 1 commit into from
Closed

Conversation

timhoffm
Copy link
Member

@timhoffm timhoffm commented Jun 10, 2019

PR Summary

PackerBase does not much. It just has an __init__ that defines a common constructor and some attributes.

I don't see a need for this base class to be visible to the end-user. By making it private we save the effort of documenting it (__init__ docstrings have to be present in the derived classes because numpydoc does not inherit __init__ docstrings if there is a class docstring).

We may also consider removing it alltogether later.

@anntzer
Copy link
Contributor

anntzer commented Jun 11, 2019

It feels more like an issue on numpydoc's side (regarding init docstring inheritance)?
As I already argued elsewhere I'm not a fan of publically inheriting private classes, but it's not a huge deal either.

@timhoffm
Copy link
Member Author

It feels more like an issue on numpydoc's side (regarding init docstring inheritance)?

That's half of the story. But even if that was working, do we need a common base class for HPacker and VPacker? If one wants to be completely accurate, one would need slightly different parameter docs for those (width vs. height). These parts of the signature cannot be really well-defined at all in the base class. Making

As I already argued elsewhere I'm not a fan of publically inheriting private classes, but it's not a huge deal either.

Semi-OT: I understand that. It's no problem if the base class does not provide any public functionality (as in this case). In general, I'm wondering how to do this correctly. For example _AxesBase, I don't want to make it public in the sense that users should not directly use it. But I would like the methods of _AxesBase to be available via it's children; in particular also in the documentation.

@timhoffm timhoffm marked this pull request as draft July 14, 2020 21:24
@timhoffm
Copy link
Member Author

timhoffm commented May 1, 2022

I don't have a strong opinion in the case of PackerBase. I'm not following up on this.

@timhoffm timhoffm closed this May 1, 2022
@anntzer
Copy link
Contributor

anntzer commented May 27, 2022

Note that a common __init__.__doc__ has been set up in #19012; I guess the numpydoc limitation has been addressed at some point in between.

@timhoffm timhoffm deleted the deprecate-PackerBase branch July 19, 2024 11:31
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.

2 participants