KaveIO / PhiK

Phi_K correlation analyzer library
Other
155 stars 28 forks source link

Feature Request: move matplotlib into extras dependency #31

Closed wwuck closed 2 years ago

wwuck commented 2 years ago

Hi, would it be possible to move matplotlib into an extras dependency? We are currently building a custom version of phik for our local production usage without the report.py module as matplotlib is a relatively large download on top of the tiny phik library and we don't need matplotlib for our use cases.

If if is too hard to split out, we can still continue with building our local wheels on each phik release.

Thanks for considering this request.

RUrlus commented 2 years ago

Hi @wwuck, I don't think placing a burden on the vast majority of users/downstream libs to update the installation is a good tradeoff for this very specific scenario.

mbaak commented 2 years ago

Hi @wwuck @RUrlus, we could think about having a dedicated requirements section without matplotlib, if that is possible. (I.e. default installation includes it.) I'm happy to look into that.

mbaak commented 2 years ago

I'm afraid I don't see an easy way to opt out of installing a (heavy) dependency, if one wants it to be present by default. setuptools only allows extra requirements, but not less.

wwuck commented 2 years ago

The only other alternative that comes to mind is doing similar to what the gitlint project did recently, and split into two packages (https://github.com/jorisroovers/gitlint/issues/162).

It's not directly equivalent, but in essence you would change the original phik package in the codebase to be a phik-core package without matplotlib dependency or reporting module by default and an extras specifying matplotlib dependency for the reporting module to use.

Then you create a new minimal phik package configuration that just has an install dependency on phik[reporting] (or phik[matplotlib] or similar naming).

Existing phik package users wouldn't need to change anything, and most likely wouldn't notice anything except an extra dependency being installed. This would allow people not needing the heavy matplotlib dependency to just install phik-core instead of the full phik package.

RUrlus commented 2 years ago

Hi @wwuck, Max and I had a chat about a possible solution and we're not keen on splitting the package at this time. However, what do you think about using an environment variable, e.g. PHIK_NO_MPL to remove Matplotlib as a dependency. The setup.py can check if this variable is set and remove Matplotlib from the INSTALL_REQUIRES list.

mbaak commented 2 years ago

@wwuck Can you work with the proposed solution?

wwuck commented 2 years ago

Sorry, using an environment variable feels like too much of a giant hacky band-aid to this problem and leaves too many ways to run into problems if the environment variable needs to be defined every time that we install a package that depends on phik across all of our projects (using virtualenv, tox, nox, vscode, docker, CI/CD, etc.).

We are happy to continue with building a custom phik package for our local pypi repository if there is no better fix available.

mbaak commented 2 years ago

Ok. We will keep following setuptools, to see if the feature to remove dependencies will get introduced. For now I'll close the issue.