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.
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. 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.
@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 scikit-package/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. scikit-package/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?
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
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.
Activity
sbillinge commented on Aug 19, 2024
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 commented on Aug 21, 2024
Solved, see diffpy/diffpy.pdfgui#169
bobleesj commented on 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 == 0Tieqiong commented on 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 scikit-package/scikit-package#108):
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. scikit-package/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 commented on 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:
References:
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 commented on Sep 5, 2024
Ok, it seems it can go either way. It probably doesn't matter that much.