pepkit / peppy

Project metadata manager for PEPs in Python
https://pep.databio.org/peppy
BSD 2-Clause "Simplified" License
37 stars 12 forks source link

Tests directory incorrectly included in package installation #478

Closed jlumpe closed 1 month ago

jlumpe commented 4 months ago

The tests/ subdirectory is included in the package distribution and is installed to the user's site-packages directory:

$ pip show -f peppy
Name: peppy
Version: 0.40.1
Summary: A python-based project metadata manager for portable encapsulated projects
Home-page: https://github.com/pepkit/peppy/
Author: Michal Stolarczyk, Nathan Sheffield, Vince Reuter, Andre Rendeiro, Oleksandr Khoroshevskyi
Author-email:
License: BSD2
Location: /home/jared/tmp/peppy-env/lib/python3.10/site-packages
Requires: numpy, pandas, pyyaml, rich, ubiquerg
Required-by:
Files:
  peppy-0.40.1.dist-info/INSTALLER
  peppy-0.40.1.dist-info/LICENSE.txt
  peppy-0.40.1.dist-info/METADATA
  peppy-0.40.1.dist-info/RECORD
  peppy-0.40.1.dist-info/REQUESTED
  peppy-0.40.1.dist-info/WHEEL
  peppy-0.40.1.dist-info/top_level.txt
  peppy/__init__.py
  peppy/__pycache__/__init__.cpython-310.pyc
  peppy/__pycache__/_version.cpython-310.pyc
  peppy/__pycache__/const.cpython-310.pyc
  peppy/__pycache__/exceptions.cpython-310.pyc
  peppy/__pycache__/parsers.cpython-310.pyc
  peppy/__pycache__/project.cpython-310.pyc
  peppy/__pycache__/sample.cpython-310.pyc
  peppy/__pycache__/simple_attr_map.cpython-310.pyc
  peppy/__pycache__/utils.cpython-310.pyc
  peppy/_version.py
  peppy/const.py
  peppy/exceptions.py
  peppy/parsers.py
  peppy/project.py
  peppy/sample.py
  peppy/simple_attr_map.py
  peppy/utils.py
  tests/__init__.py
  tests/__pycache__/__init__.cpython-310.pyc
  tests/__pycache__/conftest.cpython-310.pyc
  tests/__pycache__/test_Project.cpython-310.pyc
  tests/conftest.py
  tests/smoketests/__init__.py
  tests/smoketests/__pycache__/__init__.cpython-310.pyc
  tests/smoketests/__pycache__/test_Project.cpython-310.pyc
  tests/smoketests/__pycache__/test_Sample.cpython-310.pyc
  tests/smoketests/test_Project.py
  tests/smoketests/test_Sample.py
  tests/test_Project.py
nsheff commented 4 months ago

Thanks for raising this. I agree that's strange.

@stolarczyk did you have a reason for adding the /tests folder to the MANIFEST.in?

https://github.com/pepkit/peppy/blob/a8becea34be9d8947c43b445bc881855770679c5/MANIFEST.in#L5

stolarczyk commented 4 months ago

I may have had a reason initially, but it's probably no longer valid. I second the suggestion for removal.

jlumpe commented 4 months ago

I don't think this is just a question of MANIFEST.in, as these are modules and not just data files. Pretty sure the issue is setup.py L43, setuptools.find_packages() will include tests/ as it contains an __init__.py. I believe setuptools has default behavior to exclude common directories like tests/, docs/ etc from package discovery but only if you don't provide values for any of the package discovery options. Solution would be to remove that line entirely or change to find_packages(exclude=['tests', 'tests.*']).

nsheff commented 4 months ago

or, even better, just use packages=['peppy'], since this will only ever hold a single package.

Thanks for pointing that out.

jlumpe commented 4 months ago

@nsheff or packages=['peppy', 'peppy.*'] if you have any subpackages (not sure why setuptools doesn't provide an easy way to combine those).