Skip to content

Fold table.CustomCell into Cell. #17032

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Apr 15, 2020
Merged

Fold table.CustomCell into Cell. #17032

merged 1 commit into from
Apr 15, 2020

Conversation

anntzer
Copy link
Contributor

@anntzer anntzer commented Apr 4, 2020

We don't use plain Cells, only CustomCells, which is a subclass.
Flattening the inheritance hierarchy makes it easier to document all
parameters in a single constructor. No backcompat break as CustomCell
is left as an alias.

PR Summary

PR Checklist

  • Has Pytest style unit tests
  • Code is Flake 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

@anntzer anntzer added this to the v3.3.0 milestone Apr 4, 2020
@ImportanceOfBeingErnest
Copy link
Member

One needs to be aware of the fact that noone would construct a CustomCell by themselves anyways.
Instead, one would monkey-patch the CustomCell with a new subclass of it, like

matplotlib.table.CustomCell = CellThatCanDoMoreCoolThings

and obviously, that will break with this change. (One application would be this one, using a cell that can have a background color without edges)

@timhoffm
Copy link
Member

timhoffm commented Apr 4, 2020 •
edited
Loading

I suppose we don‘t guarantee that users can monkey-patch classes in Matplotlib.

Bit if there is real concern, we could move the functionality, but keep using CustomCell and deprecate that to use Cell instead.

@ImportanceOfBeingErnest
Copy link
Member

True. I just want to mention it, because "No backcompat break" sounds like it wouldn't annoy anyone.

@anntzer
Copy link
Contributor Author

anntzer commented Apr 4, 2020

Well, literally (nearly) any change is a backcompat break in that sense anyways, because even when adding a new feature, we may be overstepping over another method/kwarg name that someone else is using in their monkeypatched alternative class.

@timhoffm
Copy link
Member

timhoffm commented Apr 12, 2020 •
edited
Loading

This should have an API change note even though it's not a breaking change.

I advocate deprecating CustomCell to remove unnecessary clutter in the long run.

@anntzer
Fold table.CustomCell into Cell.
66fa8bb 
We don't use plain Cells, only CustomCells, which is a subclass.
Flattening the inheritance hierarchy makes it easier to document all
parameters in a single constructor.  No backcompat break as CustomCell
is left as an alias.
@anntzer
Copy link
Contributor Author

anntzer commented Apr 12, 2020

sure

@timhoffm timhoffm merged commit e55e79b into matplotlib:master Apr 15, 2020
@anntzer anntzer deleted the ccell branch April 15, 2020 08:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@QuLogic QuLogic QuLogic approved these changes

@timhoffm timhoffm timhoffm approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v3.3.0
Development

Successfully merging this pull request may close these issues.
4 participants
@anntzer @ImportanceOfBeingErnest @timhoffm @QuLogic