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

Publish this tool on PyPI #35

Open
JohnGiorgi opened this issue Aug 17, 2018 · 15 comments · Fixed by #110
Open

Publish this tool on PyPI #35

JohnGiorgi opened this issue Aug 17, 2018 · 15 comments · Fixed by #110
Assignees
Labels
enhancement New feature or request production

Comments

@JohnGiorgi
Copy link
Contributor

JohnGiorgi commented Aug 17, 2018

We need to properly package Saber, and publish it on PyPI so that it can be pip install saber. I don't know how to do this, so I need to do my homework first and then publish it. See here for some inspiration.

Resources

@JohnGiorgi JohnGiorgi added the enhancement New feature or request label Aug 17, 2018
@JohnGiorgi JohnGiorgi self-assigned this Aug 17, 2018
@cthoyt
Copy link
Contributor

cthoyt commented Jan 19, 2019

I know quite a bit about this and would be happy to help. Packaging tools that have exotically specified dependencies like this is tricky though, and I've never had good luck using dependency_links. Do you think it's possible to find more streamlined distributions (hopefully via PyPI) of the keras contrib repo and the NeuroalCoref medium model?

@JohnGiorgi JohnGiorgi pinned this issue Jan 19, 2019
@JohnGiorgi
Copy link
Contributor Author

@cthoyt Oh wow, well that would be great!

This has been the main thing holding me back from trying to stick Saber on PyPI in the first place. The documentation of both keras-contrib and NeuralCoref both instruct you to use pip install on some GitHub URL.

If there is a more streamlined distribution for either of these tools, I do not know about it. I have even opened an issue on the keras-contrib repo asking for help installing it with a setup.py file. None of advice did the trick for me.

@cthoyt
Copy link
Contributor

cthoyt commented Jan 20, 2019

PR is all done. It looks like it should work now!

@cthoyt
Copy link
Contributor

cthoyt commented Jan 20, 2019

However, it looks like pip install saber has already been snagged...

Update: I've dealt with packages whose names on pip and the thing they install are not the same. I would strongly recommend against doing that. It's a huge problem for users.

That doesn't mean that the project name has to be changed nor the repository name on GitHub (in fact, I think your acronym is very cool). What I might suggest is changing the folder in the repository where all the code lives to something else like saber_ner and also change the name in setup.py to match. Of course a new name would be up to you.

@JohnGiorgi
Copy link
Contributor Author

JohnGiorgi commented Jan 20, 2019

Okay so it looks like the PyPI FAQs mention something about claiming names from abandoned projects. A bit more digging reveals what is considered an abandoned project.

I would say the saber project currently onl PyPI satisfies the second two criteria (no activity within 12 months and no homepage listed).

It is just not clear how (or if?) I can make a request for the name saber. I could always email the owner of the current saber project and ask if they are willing to remove their project from PyPi (the last activity was in 2014 after all). I emailed the author... now we wait! the author got back to me, saber is now free for us to use!

If that doesn't work / isn't a good idea, I will try to think of a new name as you suggest. I would probably go with saber_nlp or saber_ie.

(in fact, I think your acronym is very cool)

Thanks! I was pretty proud of it haha

@cthoyt
Copy link
Contributor

cthoyt commented Jan 21, 2019

That’s really great to hear! Definitely claim it yourself ASAP. The best way would be to make an account on PyPI, then run the tox -e release command I added for you, which takes care of building and uploading to PyPI. Even if there are some issues, you can always release a new version later

@JohnGiorgi
Copy link
Contributor Author

@cthoyt Okay I ran into this issue after running tox -e release:

release create: /Users/johngiorgi/Documents/Masters/Class/natural_language_computing/project/saber/.tox/release
release installdeps: wheel, setuptools, twine >= 1.5.0
release installed: bleach==3.1.0,certifi==2018.11.29,chardet==3.0.4,docutils==0.14,idna==2.8,pkginfo==1.5.0.1,Pygments==2.3.1,readme-render
er==24.0,requests==2.21.0,requests-toolbelt==0.8.0,saber==0.0.1,six==1.12.0,tqdm==4.29.1,twine==1.12.1,urllib3==1.24.1,webencodings==0.5.1
release run-test-pre: PYTHONHASHSEED='1259105692'
release runtests: commands[0] | python setup.py -q sdist bdist_wheel
no previously-included directories found matching 'notebooks'

no previously-included directories found matching 'img'
release runtests: commands[1] | twine upload --skip-existing 'dist/*'
Enter your username: johnmgiorgi
Enter your password:
Uploading distributions to https://upload.pypi.org/legacy/
Uploading saber-0.1.0-py3-none-any.whl
100%|█████████████████████████████████████████████████████████████████████████████████████████████████| 91.9M/91.9M [01:14<00:00, 1.29MB/s]
NOTE: Try --verbose to see response content.
HTTPError: 400 Client Error: Invalid value for requires_dist. Error: Can't have direct dependency: 'keras-contrib @ git+https://www.github.
com/keras-team/keras-contrib.git' for url: https://upload.pypi.org/legacy/
ERROR: InvocationError for command '/Users/johngiorgi/Documents/Masters/Class/natural_language_computing/project/saber/.tox/release/bin/twi
ne upload --skip-existing dist/*' (exited with code 1)
_________________________________________________________________ summary _________________________________________________________________
ERROR:   release: commands failed

Any suggestions? I will try to figure it out in the meantime!

@JohnGiorgi JohnGiorgi reopened this Jan 21, 2019
@cthoyt
Copy link
Contributor

cthoyt commented Jan 21, 2019

Oh dear... Maybe one of sdist or bdist_wheel doesn't accept this type of requirement :/ Maybe you can try removing those one at a time? You'll have to delete the build and dist folders in the mean time. I might try it myself on test PyPI and see if I can get it working later.

@JohnGiorgi
Copy link
Contributor Author

I think you are correct, if I remove keras-contrib @ git+https://www.github.com/keras-team/keras-contrib.git from setup.py and run tox -e release I get the same error, just about en-cored-md this time:

release installed: bleach==3.1.0,certifi==2018.11.29,chardet==3.0.4,docutils==0.14,idna==2.8,pkginfo==1.5.0.1,Pygments==2.3.1,readme-renderer==24.0,requests==2.21.0,requests-tool
belt==0.8.0,saber==0.1.0,six==1.12.0,tqdm==4.29.1,twine==1.12.1,urllib3==1.24.1,webencodings==0.5.1
release run-test-pre: PYTHONHASHSEED='2477164563'
release runtests: commands[0] | python setup.py -q sdist bdist_wheel
no previously-included directories found matching 'notebooks'

no previously-included directories found matching 'img'
release runtests: commands[1] | twine upload --skip-existing 'dist/*'
Enter your username: johnmgiorgi
Enter your password:
Uploading distributions to https://upload.pypi.org/legacy/
Uploading saber-0.1.0-py3-none-any.whl
100%|████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████████| 91.9M/91.9M [01:07<00:00, 1.43MB/s]
NOTE: Try --verbose to see response content.
HTTPError: 400 Client Error: Invalid value for requires_dist. Error: Can't have direct dependency: 'en-coref-md @ https://github.com/huggingface/neuralcoref-models/releases/downl
oad/en_coref_md-3.0.0/en_coref_md-3.0.0.tar.gz' for url: https://upload.pypi.org/legacy/
ERROR: InvocationError for command '/Users/johngiorgi/Documents/Masters/Class/natural_language_computing/project/saber/.tox/release/bin/twine upload --skip-existing dist/*' (exit
ed with code 1)
____________________________________________________________________________________ summary _____________________________________________________________________________________
ERROR:   release: commands failed

@cthoyt
Copy link
Contributor

cthoyt commented Jan 21, 2019

Try modulating the release command in tox.ini from python setup.py -q sdist bdist_wheel to

  1. python setup.py -q bdist_wheel
  2. python setup.py -q sdist

And see if either of those work. Worst case scenario is you can remove them from the requirements, deploy anyway, and add more information to the documentation that they need to be installed separately. I hope it doesn't come to that, though.

@JohnGiorgi
Copy link
Contributor Author

JohnGiorgi commented Jan 21, 2019

Oops, I think I misunderstood your previous comment. Right so I tried both

  1. python setup.py -q bdist_wheel
  2. python setup.py -q sdist

in tox.ini. Unfortunately, I get the same issues as above.

Thanks! python setup.py -q sdist worked. Saber is now on PyPI!

Update: The release went through with tox -e release, but when I try to pip install saber on my local machine I get the following error:

Packages installed from PyPI cannot depend on packages which are not also hosted on PyPI.
saber depends on keras-contrib@ git+https://www.github.com/keras-team/keras-contrib.git

😢

@cthoyt
Copy link
Contributor

cthoyt commented Jan 21, 2019

So close... so the reason this didn’t work with bdist_wheel is that builds a binary package and it knew to get upset about the weird dependencies. I’m really surprised to see PyPI complains about this. Maybe open an issue on their github to see what they think a better solution is?

We could also engage full revolution mode and go and help get the other two packages on PyPI too 😁

@JohnGiorgi
Copy link
Contributor Author

So close... so the reason this didn’t work with bdist_wheel is that builds a binary package and it knew to get upset about the weird dependencies. I’m really surprised to see PyPI complains about this. Maybe open an issue on their github to see what they think a better solution is?

It looks like this is a conscious decision on their part (see last paragraph in this response). Basically, they don't want pip to reach out to URLs outside of PyPI when installing from PyPI.

We could also engage full revolution mode and go and help get the other two packages on PyPI too 😁

Might come down to this. I know there are some still maintaining keras-contrib so I asked in an issue there if they were planning on uploading to PyPI. Also asked a similar question for neural-coref. Depending on the response we could suggest helping them get their packages on PyPI!

In the meantime, I will take a look at alternatives to neural-coref. I only need two source files from keras-contrib so I could always just include them in saber (not a great solution obviously).

@1dot75cm
Copy link

1dot75cm commented Feb 27, 2019

You can run custom scripts via setuptools.command.install class. Full example. @JohnGiorgi

from setuptools.command.install import install
from subprocess import getoutput
...
class PostInstall(install):
    pkgs = ' git+https://www.github.com/keras-team/keras-contrib.git'\
           ' https://github.com/huggingface/neuralcoref-models/releases/download/en_coref_md-3.0.0/en_coref_md-3.0.0.tar.gz'
    def run(self):
        install.run(self)
        print(getoutput('pip install'+self.pkgs))
        #https://pip.pypa.io/en/stable/user_guide/#using-pip-from-your-program
        #subprocess.call([sys.executable, '-m', 'pip', 'install', self.pkgs])
...
setup(
...
    cmdclass={'install': PostInstall}
...
)

And then,

$ python3 setup.py sdist
$ pip install -vvv dist/package-version.zip

Please note, the wheels does not support this.

@cthoyt
Copy link
Contributor

cthoyt commented Feb 27, 2019

That's a cool solution. For the time being, it's probably okay to just remove these requirements from the pip installation and make sure that there is adequate user documentation on how to install these last two packages in the README (which goes to PyPI too)

@JohnGiorgi JohnGiorgi added this to To do in Roadmap to 0.1.0 via automation Jul 28, 2019
jacksonllee added a commit to jacksonllee/pycantonese that referenced this issue Oct 11, 2020
because a github direct link as a dependency apparently doesn't
play nice with PyPI. Relevant discussions:
pypa/pip#6301
BaderLab/saber#35
ZhanruiLiang pushed a commit to ZhanruiLiang/pycantonese that referenced this issue Oct 4, 2022
because a github direct link as a dependency apparently doesn't
play nice with PyPI. Relevant discussions:
pypa/pip#6301
BaderLab/saber#35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request production
Projects
Development

Successfully merging a pull request may close this issue.

3 participants