search

dpeerlab / Harmony

Harmony framework for connecting scRNA-seq data from discrete time points
GNU General Public License v2.0
44 stars 12 forks source link

Don’t install things in setup.py #10

Closed flying-sheep closed 4 years ago

flying-sheep commented 4 years ago

This is very bad behavior:

https://github.com/dpeerlab/Harmony/blob/531b4e8a517c7b06351630596d2d262fbb8cb011/setup.py#L19

You have to specify it as a dependency in setup() instead.

Palantir should be on PyPI too, of course.

hisplan commented 4 years ago

or we could do something like this in setup():


install_requires=[
  ...
  'Palantir@https://github.com/dpeerlab/Palantir/archive/v0.2.1.tar.gz'
],
flying-sheep commented 4 years ago

For the time being, yes. But PyPI is still much more convenient for users.

Does that syntax work? I only know git+https://...

awnimo commented 4 years ago

As we move forward with distributing Harmony through PyPI, we will definitely address this important issue, and implement better practices in the new releases.

flying-sheep commented 4 years ago

It’s dangerous. Executing pip with flags that one didn’t chose themselves (e.g. when someone always uses --user, and now that setup.py doesn’t) can result in your whole system of python packages being messed up.

hisplan commented 4 years ago

Agreed. Anyway, the syntax that I mentioned works fine (per PEP).

flying-sheep commented 4 years ago

Good to know! You learn something new every day. This syntax is probably better than git+https://...#tag=v0.2.1 which I used until now, as downloading a tarball has to be faster than cloning a git repo

ManuSetty commented 4 years ago

Fixed as part of PyPI release https://pypi.org/project/harmonyTS/