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

Move requirements.txt to setup.py #413

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from

Conversation

PromyLOPh
Copy link
Contributor

@PromyLOPh PromyLOPh commented Mar 9, 2019

This ensures only known-good dependencies are used. By ignoring new, non-working versions we can concentrate on fixing the test suite. If we got that, we can come back later and selectively upgrade dependencies, one by one, fixing issues as they come up.

Edit: Uh, lemme check https://travis-ci.org/ArchiveTeam/wpull/jobs/504035201#L1437 before merging.

Advantages:
- Single-command install (pip install .)
- Package can only be installed with known-good dependencies
warcat and youtube-dl actually need to be installed, but tests_require
only downloads the egg and sets sys.path.
@JustAnotherArchivist
Copy link
Contributor

I know that this separation is often done on purpose since requirements.txt is for deployment while setup.py lists dependencies. Meaning the former should contain tested and confirmed-working versions but the latter only minimal requirements (e.g. excluding major versions with breaking changes). In this case, the versions of lxml and sqlalchemy at least are most likely not due to breaking changes.

@PromyLOPh
Copy link
Contributor Author

@JustAnotherArchivist We can look into lxml and sqlalchemy and remove the restriction when the tests pass and we’re confident it works. I still believe setup.py is the best place for dependency information. This enables the packet manager (pip) to make smart choices (or reject conflicts) and the user can simply run pip install .. Anyway, it’s up to you.

I’ve removed the commit changing dnspython3 to dnspython, >=1.13 breaks a test. (Well, doh.)

@JustAnotherArchivist
Copy link
Contributor

Yeah, I think I agree. I just wanted to mention why this separation might currently be the case.

Is there a good way to test different versions of dependencies, e.g. on Travis CI?

@PromyLOPh
Copy link
Contributor Author

@JustAnotherArchivist
Copy link
Contributor

Yeah, I was hoping for a solution where we don't have to explicitly specify each version we want to test, but I guess that's a bit too much to ask.

chardet: Works fine with 3.0.4. No API changes
namedlist: No new release since 1.7
sqlalchemy: Works fine with 1.3.1. NO UNIT TESTS!
yapsy: Version 1.12.0 works.
Requires pip>=9 or setuptools>=24
We want automated tests. Even for unsupported versions.
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