Skip to content

Idea: copy cycler package into matplotlib sourcecode #14682

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
Closed
windelbouwman opened this issue Jul 3, 2019 · 2 comments
Closed

Idea: copy cycler package into matplotlib sourcecode #14682

windelbouwman opened this issue Jul 3, 2019 · 2 comments

Comments

@windelbouwman
Copy link

This is not a bug, but an idea / suggestion. It has been probably a topic of debate in the past, but I would like to bring this topic up again.

Given the size of the cycler package, it might be a good idea to copy the sourcecode of cycler (some 500 lines) into the matplotlib repository and get rid of this dependency. When I first read about the purpose of cycler, I was somewhat surprised that it is a seperate package. It's not as small as the javascript is-even package, but still, it's fairly small.

Advantages:

  • Less packages, so matplotlib becomes easier to re-package for distribution builders.
  • No configuration management / incompatibilities between cycler and matplotlib.
  • Easier implementation of cross package changes (i.e. changes which must be applied to matplotlib and cycler at the same time).

Disadvantages:

  • code duplication of the cycler package.

Please feel free to close this issue immediately if this was already brought up many times. Thank you for creating matplotlib! It's fantastic!
@story645
Copy link
Member

story645 commented Jul 3, 2019

If Matplotlib were to do this, I think the approach would be to include the package as a git submodule so that there's no code duplication.

But, I'm also -1 on this 'cause Matplotlib would like to shed code from its core, not add to it. I've run into the bug where the packager doesn't pick up/install cycler, and I think the fix also needs to be on the packaging side.

@tacaswell
Copy link
Member

cycler actually started life as a PR into Matplotlib (see #4258)! It was deemed useful enough that people wanted to be able to install it independently of Matplotlib, hence why it is in it's own package.

cycler has (fortunately) received very little work over the past few years (because it has just one job!). I also don't see there being any sort of change on either the cycler or Matplotlib sides that would cause any incompatibilities or require coordinated changes so while true in general, I do not think most of those advantages are persuasive here.

A bigger downside is that it is there are a number of isinstance checks in the implementation of cycler.Cycler (e.g. Cycler.__mul__) so if Matplotlib vendored cycler back you could end up with two Cycler objects that would not compose with each other.

I feel you on the less packages from a packagers point of view (I am a maintainer on a fair number of CF feedstocks), but as this dependency came in almost 4 years ago we have already paid the cost of getting it packaged by all of the major down-stream packagers. Reverting on that would likely be more hassle than maintaining the status quo.

I agree with @story645 on this and am going to close this issue.

@windelbouwman Given your question and that you are a member of rustpython, are you trying to get Matplotlib working on top of rust python (which would also imply that you have numpy working?)? Or are you packaging Matplotlib for some other stack? Either way, sounds like something cool!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@tacaswell @story645 @windelbouwman