Fix dependency handling #100
Conversation
Replace pip install from requirements.txt with a simple install from local dir (should should be synonym to python setup.py install).
|
Interesting writeup about the different responsibilities of requirements.txt and install_requires in setup.py: https://stackoverflow.com/a/33685899/10207149 |
was installed via requirements.txt before.
|
Hi @AnssiAhola and @alifhughes, please review my propsal. I might be on the wrong track with removing the rather static requirements.txt entirely. The problem with the current state of the PR it that nosetests and coverage are not documented how to install, that's an issue for submitters who want to run tests locally. What's your opinion on the responsibilites of requirements.txt and the install_requires line in setup.py? |
|
If I understand correctly, requirements.txt is more for setting up dev environment? Edit. |
|
No actually the sphinx requ file is only the additional files needed for docs contribution. I had it split into two files so to say. ok so maybe lets keep requs.txt but better versioning definitions. |
|
Oh I see. |
This reverts commit f76051e. Bring back requirements.txt and sphinx_requirements.txt. The latter is required for Sphinx docs generation via readthedocs.io.
set of required packages. Note: readthedocs.io requires to set the name of _one_ requirements file to get deps installation right. Let's try to also use this file for installing requirements for local docs building. Currently a bug in Jinja2 (or Sphinx, not sure) requires pinning Jinja2 to a version below 3.1. See sphinx-doc/sphinx#10291
|
Yes exactly those two are "extra packages" needed for development/testing. I yesterday realized while working on #99 that readthedocs.io can't build our docs anymore due to a Jinja2 import error. That reminded me that sphinx_requirements.txt I put there for a reason: In the readthedocs.io admin for our project a single requirements file needs to be set. This one. I now fixed the import error by minimalizing this file and temporary fixing the Jinja2 import by requiring a specific version. Have a look at 16ce256 I still opt for the split of a requirements file for regular code development and docs building, simply because in the past there have been issues with docs requirements and I don't want to add this complexity for contributors that would just like to submit a feature and write a test for it. You agree that this a good idea Anssi or shouild we try to find "one known to be good" requirements.txt file for all needs? Anyway I would like to postpone this decision because at the moment my priority is to get pull 99 finished and fix docs building therefore. The regular requirements.txt I now minimized to just any version of nose and coverage like you suggested: Just dev-requirements but not deployment requirements. I like this minimal approach but I am not sure if some people would expect that a |
|
Our docs building in latest/master version is now fixed :-) |
Since recently we realized that our requirements.txt is wrong anyway (#99), I propse we kick it out. Regular installs from git use the deps provided in setup.py anway.
Users/devs who want to add to our docs should then be advised to simply install additional requirements from the simplified sphinx_requirements.txt file.
Also this PR should address the security vulnerabilites detected by dependabot: https://github.com/joalla/discogs_client/security/dependabot