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

Unable to install without the ML functionality #130

Open
alexei opened this issue Mar 8, 2017 · 6 comments · May be fixed by #200
Open

Unable to install without the ML functionality #130

alexei opened this issue Mar 8, 2017 · 6 comments · May be fixed by #200

Comments

@alexei
Copy link

alexei commented Mar 8, 2017

How does one install talon without ML? I tried both pip install talon --global-option="--no-ml" and pip install talon --install-option="--no-ml" and neither seems to work (admitedly this is the first time I'm using the package options flag). Somehow it ends up trying to install all packages and fails at Scipy (either from the Fortran dependency when installing without the flag, or because option --no-ml not recognized)

Using Python 2.7.12, pip 9.0.1 on macOS

@timabbott
Copy link

The problem is I think that the options feature in pip is not super well-implemented. We were able to get that option to work, but not in a requirements.txt file.

We'd also be interested in this for Zulip (github.com/zulip/zulip/); right now we're installing off of a fork because we don't want a dependency on numpy and friends.

@timabbott
Copy link

I wonder whether a better solution would be to split out an independent package for the non-ML parts of talon. E.g. call it talon-noml, and have talon depend on that.

@ad-m
Copy link
Contributor

ad-m commented Aug 23, 2017

@timabbott , there is claw - a fork of @tictail without ML.

@imankulov
Copy link

Hey everyone! Would like to kindly ask for an update from package maintainers.

Ended up with the same issue today. Only wanted to use non-ml features of talon, but couldn't do it without extracting essential parts of it into a separate library. The claw library seems to be abandoned (updated 3 years ago and at the time of writing 179 commits behind mailgun:master).

Tried to use the --no-ml option, but ended up with several issues with it.

  1. As it was pointed out earlier, the feature is not very well maintained. The pip install talon --install-option="--no-ml applies recursively to the package itself and all its dependencies.
  2. Due to different versions of scikit-learn here and the same file here even with python setup.py install --no-ml scikit-learn will be installed.
  3. I'm not a distutil/pip expert in no way, but I have a feeling that managing dependencies with an option flag is a hacky way which is incompatible with eggs and wheels. They store package dependencies in text files xxx-dist-info/METADATA and xxx-egg-info/requires.txt and, if I'm not mistaken, let you install packages without actually resorting to execute any code from setup.py. Declaring extras sounds like a more correct way of dealing with the issue.

With all this in mind, I would see three options to solve the issue.

  1. If there is no intention to maintain the --no-ml flag, probably it makes sense to remove it from setup.py to avoid any confusion.
  2. Alternatively, ML requirements could be extracted away to extras as extras_require={"ml": ["numpy", "scipy", ...]}. It's backward incompatible of course, since now everyone who wants to use ML features, has to install it as pip install talon[ml] and update requirements correspondingly.
  3. As it was suggested, extract non-ML part to something like talon-noml and add it as a dependency.

Thanks!

@timabbott
Copy link

I'd love to see that happen. Certainly the --no-ml flag hasn't ended up being actually useful due to the requirements.txt issues I noted above. And at the same time, the features of this library that don't involve ML are really valuable, but when one isn't using the ML features, it's unfortunate to have to install the ML dependencies (because they are super slow to install and import!).

andersk added a commit to andersk/talon that referenced this issue Oct 3, 2019
The nonstandard `setup.py install --no-ml` option didn’t work
correctly with pip.  We could move the ML dependencies into an extra,
but that approach was previously rejected (mailgun#121): extras can’t be
enabled by default, so it could have been disruptive to existing
users.  Instead, we split off a new talon-core package with no ML
dependencies, and have talon re-export everything from it.

Fixes mailgun#130; fixes mailgun#131.

Signed-off-by: Anders Kaseorg <[email protected]>
@andersk andersk linked a pull request Oct 3, 2019 that will close this issue
@andersk
Copy link

andersk commented Oct 3, 2019

I wonder whether a better solution would be to split out an independent package for the non-ML parts of talon. E.g. call it talon-noml, and have talon depend on that.

I’ve implemented this in #200 (calling it talon-core), since the other extras-based solution was previously rejected in #121.

andersk added a commit to andersk/talon that referenced this issue Oct 3, 2019
The nonstandard `setup.py install --no-ml` option didn’t work
correctly with pip.  We could move the ML dependencies into an extra,
but that approach was previously rejected (mailgun#121): extras can’t be
enabled by default, so it could have been disruptive to existing
users.  Instead, we split off a new talon-core package with no ML
dependencies, and have talon re-export everything from it.

Fixes mailgun#130; fixes mailgun#131.

Signed-off-by: Anders Kaseorg <[email protected]>
andersk added a commit to andersk/talon that referenced this issue Jul 30, 2022
The nonstandard `setup.py install --no-ml` option didn’t work
correctly with pip.  We could move the ML dependencies into an extra,
but that approach was previously rejected (mailgun#121): extras can’t be
enabled by default, so it could have been disruptive to existing
users.  Instead, we split off a new talon-core package with no ML
dependencies, and have talon re-export everything from it.

Fixes mailgun#130; fixes mailgun#131.

Signed-off-by: Anders Kaseorg <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants