lucasb-eyer / pydensecrf

Python wrapper to Philipp Krähenbühl's dense (fully connected) CRFs with gaussian edge potentials.
MIT License
1.94k stars 413 forks source link

Automate tests for eigen libary. #48

Closed MarvinTeichmann closed 6 years ago

MarvinTeichmann commented 6 years ago

This patch modifies the file test_eigen.py to become a proper py.test file. Three additional tests for value and type errors have been added to the two existing ones.

It is now possible to run all tests by calling py.test -v in the project root dir or py.test -v test_eigen.py. The file eigen.so needs to be in the path in order for the tests to succeed. This can be archived by running python setup.py build_ext -i && cd pydensecrf in the project root dir.

lucasb-eyer commented 6 years ago

First off, that's pretty cool, thanks for contributing tests!

I guess I'm doing something wrong though. I followed your instructions: python setup.py build_ext -i && cd pydensecrf, and then I run py.test -v and I get the following error:

...
test_eigen.py:2: in <module>
    import eigen as e
E   ModuleNotFoundError: No module named 'eigen'
...

Same error no matter which folder I run the command from. I do have a eigen.cpython-36m-x86_64-linux-gnu.so file in the pydensecrf folder, and even adding a symlink called eigen.so doesn't help.

When I'm in the pydensecrf folder and just do import eigen in a regular python repl, the import does work.

I don't know enough of py.test to solve this myself and need your help. But note that I do have one very simple standard python unittest in tests/test_utils.py and it might make sense moving your added test over there too.

MarvinTeichmann commented 6 years ago

Thanks for the fast response. I have changed the import statement from import eigen as e to import pydensecrf.eigen as e. Now it is possible to import eigen if the (compiled) package pydensecrf can be found. Running python setup.py install should make sure that this is the case.

Importing the entire package actually also looks like the cleaner solution to me, as this lifts the requirement of test_eigen.py beeing in the same folder as eigen.so. Did not gave this much though beforehand, just copied the existing code and added the pytest directives.

I don't know enough of py.test to solve this myself and need your help.

I am not a py.test expert either. However in theory py.test -v test_eigen.py should work fine if and only if python test_eigen.py works. Py.test is designed to be as lightweight as possible not adding any overhead to the testing process.

MarvinTeichmann commented 6 years ago

I have now also touched the file test.py and added the py.test directives. Especially testing that the code raises an the exceptions looks much cleaner with pytest.

lucasb-eyer commented 6 years ago

Wow, it's been ages! :blush: I've merged this as it all works now, and added a note at the end of the README. Thanks again for your effort!