Skip to content
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

VIS: register_matplotlib_converters is exposed through plotting backend #27152

Open
jorisvandenbossche opened this issue Jun 30, 2019 · 2 comments
Labels
Internals Related to non-user accessible pandas implementation Needs Discussion Requires discussion from core team before further action Visualization plotting

Comments

@jorisvandenbossche
Copy link
Member

With the recent plotting backend refactor, the pandas.plotting.register_matplotlib_converters is now referring to the "general" register which calls the plotting backend, instead of just being the matplotlib register function.

It probably doesn't make much sense to expose this through the plotting backend?
It is maybe done for consistency (everything goes through the plotting backend, also internally), but personally I found this indirection confusing when looking at the source code (for this case).

cc @datapythonista

@datapythonista datapythonista added Needs Discussion Requires discussion from core team before further action Visualization plotting labels Jul 4, 2019
@datapythonista
Copy link
Member

I agree, more than for consistency was done to be able to fully split the matplotlib code into a separate module (without doing too many tricky things).

I'm unsure whether registering converters is useful for any backend, and it may makes sense to simply change the name of the function. Or if users should call the function directly from the backend (the backend is currently private pandas.plotting._matplotlib, so this would probably require making it public).

This is being discussed in #26747

@jorisvandenbossche
Copy link
Member Author

I personally don't think it makes sense for backends to have this (and if they have something similar, I don't think it should necessarily be exposed through pandas).

I wouldn't move the public API, the public name already has "matplotlib" in it, so it is rather clear it is for the matplotlib backend.

@jorisvandenbossche jorisvandenbossche added this to the 0.25.0 milestone Jul 6, 2019
@WillAyd WillAyd mentioned this issue Jul 18, 2019
@WillAyd WillAyd removed this from the 0.25.0 milestone Jul 18, 2019
@mroeschke mroeschke added the Internals Related to non-user accessible pandas implementation label Jul 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Internals Related to non-user accessible pandas implementation Needs Discussion Requires discussion from core team before further action Visualization plotting
Projects
None yet
Development

No branches or pull requests

4 participants