Use absolute imports and remove __init__.py from tests. #160
Conversation
This is recommended practice, and is more explicit. * hiredis/__init__.py: Replace relative import with absolute imports.
Pytest tests are typically not modules. Having an __init__.py there causes the tests to run against the source directory hiredis module, instead of a version installed and available on Python's path, which can be confusing. * tests/__init__.py: Delete file. * .github/workflows/freebsd.yaml (jobs): Invoke pytest via 'python -m pytest'. * .github/workflows/integration.yaml (jobs): Likewise. * README.md: Likewise.
Apteryks
force-pushed
the
use-absolute-imports
branch
from
9d1e906 to
671720a
Compare
| @@ -45,7 +45,7 @@ jobs: | |||
| pip install -U pip setuptools wheel | |||
| pip install -r dev_requirements.txt | |||
| python setup.py build_ext --inplace | |||
| pytest | |||
| python -m pytest | |||
There was a problem hiding this comment.
Given that these work in the root virtualenv.. I don't think we need this
| @@ -43,5 +43,5 @@ jobs: | |||
| /usr/local/bin/pip install -U pip setuptools wheel | |||
| /usr/local/bin/pip install -r dev_requirements.txt | |||
| /usr/local/bin/python3.9 setup.py build_ext --inplace | |||
| pytest | |||
| python -m pytest | |||
There was a problem hiding this comment.
Given that these work in the root virtualenv.. I don't think we need this.
|
Why the absolute import need in CI? Those environments are controlled. |
Hi! The fully absolute imports are always better than relative ones; it's a good practice, recommended in PEP8:
As for the Thanks! |
|
@Apteryks This library is definitely small enough that I agree. Obviously when bringing PEP8, we also end up with the PEP-328 rationale. But again, small project, I'm convinced. |
This makes it possible to run the test suite against an installed hiredis-py.