search

angelolab / Nimbus

Other
12 stars 1 forks source link

Added seeds to tests and changed conftest to work on my machine #60

Closed JLrumberger closed 1 year ago

JLrumberger commented 1 year ago

Hi @srivarra, I got the tests to run on my machine and here's a PR that has the necessary changes. I'd be happy to get some feedback from you.

What is the purpose of this PR?

Purpose of this PR is to make the tests run on my local device , without installing the codebase as a package. Installing the module is not desired, because it makes debugging more complicated. If installed, Python would import modules from the site-packages instead of the actuall code base, which prevents the breakpoints in other parts of the library from stopping code execution while debugging. To make it work without installing, I needed to set PYTHONPATH variables for the tests and the src folders. Also I got some errors, because the code used all available GPUs, thus I hide them for the tests.

How did you implement your changes

I added CUDA_VISIBLE_DEVICES="-1" and sys paths in conftest.py so that all tests run on my local device without installing cell_classification as a library. I also added a few np.random.seed(42) in one of the test scripts to make tests more reproducible.

Remaining issues

I am not sure if changing conftest.py is the right way to get the tests to run on my machine. I could also add a .env file, but then I need to see how to add this .env to the workspace settings in my IDE. I went for changing conftest.py because it was the easiest way. Is there a best practice here?

JLrumberger commented 1 year ago

Talked to @srivarra and tried to get around adding the path variables in conftest.py by doing pip install -e .[test] which installs the module in editable mode. This threw path errors when running pytest -c tests, so I reverted back to adding the paths in conftest. Since it doesn't break anything in the CI and let's the test run on my machine, I think we can merge it in.