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

Add Unit Tests #38

Open
bryankim96 opened this issue Jul 17, 2018 · 5 comments
Open

Add Unit Tests #38

bryankim96 opened this issue Jul 17, 2018 · 5 comments

Comments

@bryankim96
Copy link
Collaborator

Although other priorities (reorganizing the code, implementing new functionality, and making the project accessible and available for outside use) have been our primary focus so far, as the codebase becomes larger and we consider the possibility of outside contributions it seems like a good idea to begin looking seriously at implementing tests for maintaining and monitoring the code quality.

The project is relatively small and there is no need for the sort of serious testing that is used in more complicated software. However, with several interacting components, it seems like a few simple tests may be appropriate and helpful. Testing portions of the code in isolation should make it easier to recognize bugs and easier to test new changes without having to run the full training pipeline and search for errors by hand. Tests may also be helpful as a way of evaluating pull requests and preventing code regression/breaking changes.

The framework for testing and continuous integration with pytest and TravisCI is already in place, all that remains to be written are the tests themselves. The overall approach was based on the implementation of tests in ctapipe, which seems like it may be a helpful example.

Maybe we can discuss below what (if any) tests might be appropriate/helpful and who would be willing to write them.

@aribrill
Copy link
Collaborator

As a first step, the Travis CI, Coveralls, and Landscape badges in the Readme seem to have been broken by moving the repo. I'm not sure why, maybe the owner needs to be changed to ctlearn-project? @bryankim96 could you look into this?

@bryankim96
Copy link
Collaborator Author

The badges should now be working. However, I have noticed it often takes quite some time for the initial check to be completed. So, it could be a little while before some of the badges begin showing up-to-date information.

@aribrill
Copy link
Collaborator

Travis CI is now working, but I could not get Coveralls or Landscape to sync properly. I've removed them for now. I also tried installing Codecov to replace Coveralls but have removed it for now too since this is better handled after the v0.2.0 release. While it looks like Landscape provides useful and interesting code quality feedback, I found it extremely buggy and poorly documented.

@Jsevillamol
Copy link
Collaborator

Jsevillamol commented Sep 21, 2018

Some ideas for tests of ImageMapper

def test_image_mapping_one_channel():
    img_mapper = ImageMapper()
    
    for tel in img_mapper.num_pixels:
        num_pixels = img_mapper.num_pixels[tel] + 1
        in_ = np.random.rand(num_pixels, 1)
        out = img_mapper.map_image(in_, tel)
    
        assert out.shape[0:2] == img_mapper.image_shapes[tel]
        assert out.shape[2] == 1

def test_image_mapping_two_channels():
    img_mapper = ImageMapper()
    
    for tel in img_mapper.num_pixels:
        num_pixels = img_mapper.num_pixels[tel] + 1
        in_ = np.random.rand(num_pixels,2)
        out = img_mapper.map_image(in_, tel)
    
        assert out.shape[0:2] == img_mapper.image_shapes[tel]
        assert out.shape[2] == 2

Is this what you guys are thinking of?

@qi-feng
Copy link
Collaborator

qi-feng commented Sep 21, 2018

This sounds good. I can make relevant changes in the rest of this piece of code. Relevant config should also be passed and read.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants