search

plasticityai / magnitude

A fast, efficient universal vector embedding utility package.
MIT License
1.63k stars 120 forks source link

WTF? #62

Closed aborsu closed 5 years ago

aborsu commented 5 years ago

First of all thank you for making magnitude available openly, and please don't take this as an attack, it is just food for thoughts. The title sums up my reaction when I tried to install the latest version of magnitude. (I've been using version 0.1.13 for a while now)

I work in a corporate environment with an internal pypi proxy and some somewhat stringent checks on packages and stuff. All our packages and dependencies are scanned for license issues, and known code smells/bugs as well as against a list of known CVE. I tried installing the latest version of magnitude and after downloading the wheel it just hanged. After a few failed attempts I tried to download the archive and install it manually and I started seeing all these calls to aws and pypi which of course would fail since I'm cutoff from the internet.

I know that my issue isn't yours, but ... not cool guys, setup.py has its semantics and hijacking it like that ... well suffice to say I will probably have a call asking about all these connection attempts to blacklisted urls.

Anyway, not sure what can be done about it, as I didn't spend the time to understand everything that was going on but I still wanted to let you know that you are breaking at least this lone developer's project by doing so.

AjayP13 commented 5 years ago

Hi,

Sorry for the inconvenience, we've never come across someone using the package in a monitored network environment before.

Yes, we do use setup.py to pull pre-compiled wheels from AWS S3. The reason why is because we compile the wheels for sub-dependencies (like numpy) as well and bundle them all up. Normally, setup.py doesn't let you do this. We do this because with all of the different environments, operating systems, C/C++ toolchains people have installed, installation often fails due to a sub-dependency compilation error. So we decided to pre-compile everything for Windows, Linux, and Mac. This has worked great for most users.

Don't think we will reverse this, but we do have a way to install without using anything pre-compiled:

SKIP_MAGNITUDE_WHEEL=1 pip3 install pymagnitude -vvvv

This should force it to compile everything on your system, without downloading any pre-compiled wheels. Alternatively, you can always just clone the source repository and manually install the sub-dependencies as peer dependencies.

ParikhKadam commented 5 years ago

Hey @AjayP13 .. You got online after a long time. Just a small question. Are you still maintaining this repo? What are your future plans on this?

aborsu commented 5 years ago

@AjayP13 thank you for the reply. I understand where you are coming for, and for things like that we use conda internally. If I could offer something, it would be to package pymagnitude on conda-forge for those who want to use pre-compiled packages and leave the pip install pure.

Is that something you would accept if I offered a pull-request or would I be wasting my time?

AjayP13 commented 5 years ago

@ParikhKadam Yes, we plan to keep this alive, but we don't have that much time to maintain this, since this project is free. Eventually, I will get around to knocking out some of the long-standing issues. We would appreciate any other maintainers stepping in and helping clear out the low-hanging fruit. If anyone is interested in helping maintain, send me an e-mail at ajay@plasticityai.com and I can give some contributor access to the GitLab repository (of which this GitHub repository is a mirror of) and can merge in any PRs after review.

@aborsu We do accept PRs, but I'm hesitant to change the pip install process at this point since we've had it this way for a while. I think you could just as easily fork the repository with a live mirror (to pull in updates) and add your own deploy to an internal PyPI repository by replacing the setup.py file? Maybe that would work for your setup?

We are a two-person startup here :), so we don't have a lot of time on our hands for open-source with some of the other work we do, so I'm trying to avoid making large sweeping changes that will require more maintenance.

AjayP13 commented 5 years ago

@ParikhKadam I created #63 to discuss maintenance as to not hijack this thread.