persephone-tools / persephone

A tool for automatic phoneme transcription
Apache License 2.0
157 stars 26 forks source link

Write unit tests for all the pure functions. #57

Open oadams opened 6 years ago

shuttle1987 commented 6 years ago

I think related to this is changing some functions to take parameters that currently rely on global state.

oadams commented 6 years ago

Yeah, agreed.

oadams commented 6 years ago

When you do this. Ignore for now run.py, results.py, and lattice.py and of course anything that is already covered by tests (see the coveralls page, as well as what is under pytest.mark.experiment tags in persephone/test/experiments/ as per #135).

I think related to this is changing some functions to take parameters that currently rely on global state.

Yes, this would be great. I think addressing this issue and #135 now is a good move as it can give you a chance to familiarize yourself with the functionality of the code and spot places to refactor and clean things up. This should be useful in guiding some of the API decisions we'll need to make soon when revisiting GUI stuff.

shuttle1987 commented 6 years ago

197 gets us closer to having test coverage of all the important paths. Might be good to round up the loose ends sometime soon.