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

fix setup.py keyword #30

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

fix setup.py keyword #30

wants to merge 2 commits into from

Conversation

renzmann
Copy link

@renzmann renzmann commented Apr 16, 2022

I ran into this issue trying to pip install this package. The issue is not that fitz isn't listed, but the keyword for requires should actually be install_requires. Without this keyword, pip actually won't install transitive dependencies.

Since the original installation instructions recommended a git clone followed by pip install, I modified the README to demonstrate a faster installation that combines both steps, assuming the install_requires keyword is in place. Also, as already mentioned by another user, having a requirements.txt that doesn't pin versions is duplicitous of the install_requires, but I'll leave that to you to remove if you want.

Furthermore, this seems like a good package to go up on PyPI, so that users can simply pip install termpdf.py. If there's interest for that, I'd be happy to contribute a separate PR that would move the repo to setuptools or poetry that would simplify the build/distribution process a lot.

To see an example of the installation I wrote in the README.md, you could try it from my branch:

pip install git+https://github.com/renzmann/termpdf.py

@karlb
Copy link

karlb commented Apr 24, 2022

This solves #17 for me. Now I can install it with pipx install git+https://github.com/renzmann/termpdf.py while it failed with ModuleNotFoundError: No module named 'fitz' before. Thanks @renzmann! I also consider than changed installation instructions an improvement, so I suggest merging this PR.

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 this pull request may close these issues.

2 participants