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

Adding tests with pytest #206

Open
wants to merge 18 commits into
base: master
Choose a base branch
from
Open

Adding tests with pytest #206

wants to merge 18 commits into from

Conversation

dkazanc
Copy link
Collaborator

@dkazanc dkazanc commented May 14, 2024

CuPy requires for CUDA kernels to be present during the build. I've modified the build.sh and bld.bat files to do that.

@paskino A question about a CuPy dependency. It can be optional for the user to install CuPy or not, but I think I'll add the tests so they run if it is present in the environment (TODO).
Do you have a some kind of reusable external environment in your CI where you run tests for the toolkit? Could you install CuPy there? thanks

@dkazanc dkazanc changed the title Copying CuPy kernels during the build Adding tests with pytests Jun 10, 2024
@dkazanc dkazanc changed the title Adding tests with pytests Adding tests with pytest Jun 10, 2024
@dkazanc dkazanc marked this pull request as ready for review July 1, 2024 21:19
@dkazanc dkazanc requested a review from paskino July 1, 2024 21:19
@dkazanc
Copy link
Collaborator Author

dkazanc commented Jul 1, 2024

@paskino this PR covers #206 #208 and #207 issues.

  • CuPy modules are getting installed correctly during conda build
  • unittest replaced by pytests and extended to 3D (additional --runcupy flag needed to run CuPy tests and switched off by default)
  • Lena image is replaced by Pepper
  • All Demos have been updated accordingly

@paskino
Copy link
Contributor

paskino commented Jul 4, 2024

@dkazanc
Copy link
Collaborator Author

dkazanc commented Jul 5, 2024

In CIL we run some tests with the regularisation toolkit, see https://github.com/TomographicImaging/CIL/blob/master/Wrappers/Python/test/test_PluginsRegularisation.py

That should still work I guess.

CuPy is not a pre-requisite in our CI https://github.com/TomographicImaging/CIL/blob/b3d8ffeeec9413a3b54264e4097a445d2ef5b88b/recipe/meta.yaml#L33-L42

I haven't added CuPy in the list of dependencies, it is optional. The CuPy tests require an additional flag. The CuPy import modules are also independent, so the non-CuPy users shouldn't notice any difference.

@dkazanc
Copy link
Collaborator Author

dkazanc commented Aug 28, 2024

hi, any progress with this PR? As with minimal dependencies package I'd also like to add a PyPi uploading, will do in a separate PR after this merged. thanks

@dkazanc
Copy link
Collaborator Author

dkazanc commented Nov 28, 2024

Hi @paskino, is it OK to merge this please? CuPy is not a required dependency, but optional. This this PR covers #206 #208 and #207 issues. Thanks

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

Successfully merging this pull request may close these issues.

2 participants