Closed tkkuehn closed 2 years ago
- Right now all templates are for humans, but I think we are close to adding in non-human templates as well. Should this be a separate test (might be related to point 2) or should this single test be more agnostic?
I think this should either be a separate test or this test should be extended when we add non-human templates.
- How would we handle testing in the event that the fiducials are not named the same across species?
The validation logic in model.py
will need to handle that, and the tests should use the appropriate function for the templates they're testing.
I think this should either be a separate test or this test should be extended when we add non-human templates.
Fair - in this case, how would you feel about renaming test_all_templates
to test_human_templates
(line 8)?
The validation logic in model.py will need to handle that, and the tests should use the appropriate function for the templates they're testing.
Ahh yeah, been a while - forgot thats there. And we know it works for NHPs at least (I think fiducials are named the same).
Looks good to me otherwise.
How would you feel about renaming
test_all_templates
totest_human_templates
(line 8)?
Done
Proposed changes
Add a test for template FCSV validity and (eventually) fix the broken templates
Types of changes
What types of changes does your code introduce? Put an
x
in the boxes that applyChecklist
Put an
x
in the boxes that apply. You can also fill these out after creating the PR. If you are unsure about any of the choices, don't hesitate to ask!black
with the-l 79
flag.Notes
All PRs will undergo the unit testing before being reviewed. You may be requested to explain or make additional changes before the PR is accepted.
_PR template was adopted from appium_