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

setuptools-built source distributions not including tests #89

Closed
Tieqiong opened this issue Aug 19, 2024 · 6 comments
Closed

setuptools-built source distributions not including tests #89

Tieqiong opened this issue Aug 19, 2024 · 6 comments

Comments

@Tieqiong
Copy link
Contributor

Tieqiong commented Aug 19, 2024

I'm working on diffpy.pdfgui and it run tests on its diffpy dependencies. It worked fine when I installed 3.1.0 from Condaforge but if I install diffpy.structure==3.2.0 using pip it gives errors on not finding diffpy.structure.tests.

I checked the released files of 3.2.0 from Github. In diffpy.structure.egg-info/SOURCES.txt there's no src/diffpy/structure/tests or any subdirectory listed. I compared it with the egg-info SOURCES from diffpy.structure==3.1.0 on pypi and there indeed exists tests directory files.

I also looked at the other packages we released during summer. For diffpy.utils and diffpy.pdffit2 (1.4.3 during Jan, so not exactly this summer) there exists tests in SOURCES.txt and I can import them properly. For diffpy.pdfmorph and diffpy.labpdfproc there's no tests in SOURCES.txt and I can't import them.

@sbillinge So I guess this indicate for some of the releases setuptools is not discovering all of the files as part of the project’s source. This might be OK because tests dir is not part of the functional side of the package, but it's causing issue for diffpy.pdfgui as it struggles to run tests on its dependencies.

@sbillinge
Copy link
Contributor

Thanks for this. In many packages we have a exclude ["...tests"] line in pyproject.toml that may be causing this. We could try and remove that in the case of pdffit2? Please can you discuss with Sam and/or Andrew?

@Tieqiong
Copy link
Contributor Author

Solved, see diffpy/diffpy.pdfgui#169

@bobleesj
Copy link
Contributor

bobleesj commented Sep 4, 2024

@Tieqiong. I wanted to confirm that the behavior with diffpy.structure.tests is as expected with pyproject.toml configured. I verified that diffpy.structure==3.2.1/0 has no test files in the PyPI source distribution (sdist) from which the conda package is built.

Furthermore, I believe it is another source of conda-forge recipe CI failing when run_test.py is executed to find the test directory, which was excluded as @sbillinge mentioned above.

if __name__ == "__main__":
    package_directory = '{{ cookiecutter.module_name }}'
    module = importlib.import_module(package_directory)
    module_path = Path(module.__file__).parent
    test_location = module_path / 'tests' <------- No "tests" folder
    exit_code = pytest.main([str(test_location), "-v"])
    assert exit_code == 0

@Tieqiong
Copy link
Contributor Author

Tieqiong commented Sep 4, 2024

@bobleesj I don't know how the feedstock work, but for diffpy.structure (and probably all the cookiecutted packages) the idea is (according to Simon from Billingegroup/scikit-package#108):

I'm general, we want to deliver to users the smallest bundle that still has everything they might need

so we are not including tests and other non-necessary things into distributions. This could be very problematic if you try to find tests from the installed package, as it's not included (the only exception is you clone the repo and -e install so that you'll have the tests).

This's also related to another discussion with Simon about what exactly we want to test, also implied by one of Simon's responds in conda-forge/diffpy.structure-feedstock#16. I will record the discussion somewhere in the cookiecutter repo so you can look at it if you want. Billingegroup/scikit-package#113

In term of the conda-forge test not working, is it possible to directly lint the test code in the original repo, or copy the tests there and run them separately?

@bobleesj
Copy link
Contributor

bobleesj commented Sep 5, 2024

Q. Why don't we include test files in sdist?

If test files are not included in the sdist, conda-forge (or any other packaging system) won’t be able to run tests on the installed package. If tests are included like in diffpy.utils, all works, using run_test.py.

I checked the sdist files of pandas, matplotlib, and numpy:

  • pandas: Test files are included in the sdist and tests are run in conda-forge in cf-CI
  • numpy: Test files are included in the sdist and tests are run in conda-forge in cf-CI
  • matplotlib: Test files are not included in the sdist, so only import tests are run in cf-CI

References:

I'm general, we want to deliver to users the smallest bundle that still has everything they might need

So based on the case studies above, perhaps, we have one more audience type besides the end-users, us, the maintainers. @sbillinge Please advise adding test files to sdist.

@sbillinge
Copy link
Contributor

Ok, it seems it can go either way. It probably doesn't matter that much.

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

No branches or pull requests

3 participants