search

fatiando / pooch

A friend to fetch your data files
https://www.fatiando.org/pooch
Other
628 stars 75 forks source link

Add testing data to the package distributions #421

Closed avalentino closed 5 months ago

avalentino commented 5 months ago

The test code pooch/tests is installed but he data in pooch/tests/data are not. This makes it impossible to run tests on the installed package.

avalentino commented 5 months ago

Good catch!

Thanks

kloczek commented 5 months ago

Why at all test suite is distributed win .whl at all? 🤔 Better would be move test suite with its data to tests/ and distribute only in sdist.

avalentino commented 5 months ago

@kloczek to me it makes totally sense to distribute test s and test data in sdist. What could be an option, IMHO, is to not install them (i.e. do not include them in the .whl).

kloczek commented 5 months ago

test suite should not be distributed as part of the installable resources. Currently it is included

Here is pep517 build output: ```console + /usr/bin/python3 -sBm build -w --no-isolation * Getting build dependencies for wheel... running egg_info creating pooch.egg-info writing pooch.egg-info/PKG-INFO writing dependency_links to pooch.egg-info/dependency_links.txt writing requirements to pooch.egg-info/requires.txt writing top-level names to pooch.egg-info/top_level.txt writing manifest file 'pooch.egg-info/SOURCES.txt' reading manifest file 'pooch.egg-info/SOURCES.txt' reading manifest template 'MANIFEST.in' no previously-included directories found matching '.github' no previously-included directories found matching 'data' no previously-included directories found matching 'env' no previously-included directories found matching 'paper' no previously-included directories found matching 'tools' warning: no previously-included files found matching '.*.yml' warning: no previously-included files found matching '.*rc' warning: no previously-included files found matching 'Makefile' warning: no previously-included files found matching '.gitignore' warning: no previously-included files found matching '.gitattributes' warning: no previously-included files found matching 'environment.yml' warning: no files found matching 'pooch/tests/data' adding license file 'LICENSE.txt' adding license file 'AUTHORS.md' writing manifest file 'pooch.egg-info/SOURCES.txt' * Building wheel... running bdist_wheel running build running build_py creating build creating build/lib creating build/lib/doc copying doc/conf.py -> build/lib/doc creating build/lib/pooch copying pooch/__init__.py -> build/lib/pooch copying pooch/downloaders.py -> build/lib/pooch copying pooch/hashes.py -> build/lib/pooch copying pooch/processors.py -> build/lib/pooch copying pooch/utils.py -> build/lib/pooch copying pooch/core.py -> build/lib/pooch copying pooch/_version.py -> build/lib/pooch creating build/lib/pooch/tests copying pooch/tests/__init__.py -> build/lib/pooch/tests copying pooch/tests/test_hashes.py -> build/lib/pooch/tests copying pooch/tests/test_integration.py -> build/lib/pooch/tests copying pooch/tests/test_processors.py -> build/lib/pooch/tests copying pooch/tests/test_utils.py -> build/lib/pooch/tests copying pooch/tests/test_version.py -> build/lib/pooch/tests copying pooch/tests/utils.py -> build/lib/pooch/tests copying pooch/tests/test_core.py -> build/lib/pooch/tests copying pooch/tests/test_downloaders.py -> build/lib/pooch/tests running egg_info writing pooch.egg-info/PKG-INFO writing dependency_links to pooch.egg-info/dependency_links.txt writing requirements to pooch.egg-info/requires.txt writing top-level names to pooch.egg-info/top_level.txt reading manifest file 'pooch.egg-info/SOURCES.txt' reading manifest template 'MANIFEST.in' no previously-included directories found matching '.github' no previously-included directories found matching 'data' no previously-included directories found matching 'env' no previously-included directories found matching 'paper' no previously-included directories found matching 'tools' warning: no previously-included files found matching '.*.yml' warning: no previously-included files found matching '.*rc' warning: no previously-included files found matching 'Makefile' warning: no previously-included files found matching '.gitignore' warning: no previously-included files found matching '.gitattributes' warning: no previously-included files found matching 'environment.yml' warning: no files found matching 'pooch/tests/data' adding license file 'LICENSE.txt' adding license file 'AUTHORS.md' writing manifest file 'pooch.egg-info/SOURCES.txt' installing to build/bdist.linux-x86_64/wheel running install running install_lib creating build/bdist.linux-x86_64 creating build/bdist.linux-x86_64/wheel creating build/bdist.linux-x86_64/wheel/doc copying build/lib/doc/conf.py -> build/bdist.linux-x86_64/wheel/doc creating build/bdist.linux-x86_64/wheel/pooch copying build/lib/pooch/__init__.py -> build/bdist.linux-x86_64/wheel/pooch copying build/lib/pooch/downloaders.py -> build/bdist.linux-x86_64/wheel/pooch copying build/lib/pooch/hashes.py -> build/bdist.linux-x86_64/wheel/pooch copying build/lib/pooch/processors.py -> build/bdist.linux-x86_64/wheel/pooch copying build/lib/pooch/utils.py -> build/bdist.linux-x86_64/wheel/pooch copying build/lib/pooch/core.py -> build/bdist.linux-x86_64/wheel/pooch copying build/lib/pooch/_version.py -> build/bdist.linux-x86_64/wheel/pooch creating build/bdist.linux-x86_64/wheel/pooch/tests copying build/lib/pooch/tests/__init__.py -> build/bdist.linux-x86_64/wheel/pooch/tests copying build/lib/pooch/tests/test_hashes.py -> build/bdist.linux-x86_64/wheel/pooch/tests copying build/lib/pooch/tests/test_integration.py -> build/bdist.linux-x86_64/wheel/pooch/tests copying build/lib/pooch/tests/test_processors.py -> build/bdist.linux-x86_64/wheel/pooch/tests copying build/lib/pooch/tests/test_utils.py -> build/bdist.linux-x86_64/wheel/pooch/tests copying build/lib/pooch/tests/test_version.py -> build/bdist.linux-x86_64/wheel/pooch/tests copying build/lib/pooch/tests/utils.py -> build/bdist.linux-x86_64/wheel/pooch/tests copying build/lib/pooch/tests/test_core.py -> build/bdist.linux-x86_64/wheel/pooch/tests copying build/lib/pooch/tests/test_downloaders.py -> build/bdist.linux-x86_64/wheel/pooch/tests running install_egg_info Copying pooch.egg-info to build/bdist.linux-x86_64/wheel/pooch-1.8.2-py3.10.egg-info running install_scripts creating build/bdist.linux-x86_64/wheel/pooch-1.8.2.dist-info/WHEEL creating '/home/tkloczko/rpmbuild/BUILD/pooch-1.8.2/dist/.tmp-9q64we78/pooch-1.8.2-py3-none-any.whl' and adding 'build/bdist.linux-x86_64/wheel' to it adding 'doc/conf.py' adding 'pooch/__init__.py' adding 'pooch/_version.py' adding 'pooch/core.py' adding 'pooch/downloaders.py' adding 'pooch/hashes.py' adding 'pooch/processors.py' adding 'pooch/utils.py' adding 'pooch/tests/__init__.py' <<<<<<=== FROM HERE adding 'pooch/tests/test_core.py' adding 'pooch/tests/test_downloaders.py' adding 'pooch/tests/test_hashes.py' adding 'pooch/tests/test_integration.py' adding 'pooch/tests/test_processors.py' adding 'pooch/tests/test_utils.py' adding 'pooch/tests/test_version.py' adding 'pooch/tests/utils.py' adding 'pooch-1.8.2.dist-info/AUTHORS.md' adding 'pooch-1.8.2.dist-info/LICENSE.txt' adding 'pooch-1.8.2.dist-info/METADATA' adding 'pooch-1.8.2.dist-info/WHEEL' adding 'pooch-1.8.2.dist-info/top_level.txt' adding 'pooch-1.8.2.dist-info/RECORD' removing build/bdist.linux-x86_64/wheel Successfully built pooch-1.8.2-py3-none-any.whl ```

Looks like easiest way to do that would be just move pooch/tests/ to tests/ If test suite is not using relative imports .. if it uses then those relative imports needs to be dropped (using relative imports is always is nothing more than asking for troubles ..).

kloczek commented 5 months ago

Just checked source code and relative imports are in use 😞 In first file pooch/tests/utils.py I found:


from .. import __version__ as full_version
from ..utils import check_version, get_logger
santisoler commented 5 months ago

Distributing tests with the source code was a design decision that dates back to the origin of the package. This was intended to allow users to run tests on the installed version of the package without the need to clone the repository. We saw value on it back in the days. We even included instructions on how to run the tests after installation (see the old docs: https://www.fatiando.org/pooch/v1.0.0/install.html#testing-your-install). The relative imports were also intended: since tests were part of the code base, it made sense to use relative imports as we would do in any submodule of the package.

Nonetheless, we've been discussing about removing the test suite from the package since users rarely test the code after installation, and we would like to reduce the size of the packages that ppl download, specially since Pooch is being widely used by many packages in the community. Check out https://github.com/fatiando/community/issues/154, https://github.com/fatiando/pooch/pull/423, https://github.com/fatiando/pooch/issues/416 and https://github.com/fatiando/pooch/pull/427 for more details.

I appreciate your suggestions on this matter. Although, I don't think these design decisions were bad in nature. We wanted to provide our users with a feature (run tests after installation), which now we are reconsidering in favor of reducing installation size. There's no PEP rule against shipping tests along with code (in fact many heavily used scientific Python packages ship tests. See Numpy, matplotlib, scikit-learn as examples). In fact, including tests in the code of the package is one of the test layouts recommended by pytest. Therefore, the decisions to include the tests folder within the sources or outside of it, or to ship them or not are mostly tied to the preferences and needs of the maintainers.