Add mypy and various clean up #301
Conversation
mypy.ini
Outdated
| disallow_incomplete_defs = False | ||
| check_untyped_defs = True | ||
| no_implicit_optional = True | ||
| exclude = tests|demo|gui|scripts|display.paradigm|language|signal.model|task.control|helpers.parameters |
There was a problem hiding this comment.
Locally I get mypy errors for any psychopy imports that I have to ignore using # type: ignore. Is this something that can be added here?
There was a problem hiding this comment.
This configuration will silence that error! mypy, much to my dismay, doesn't allow inline exclusions, but you can disable it by file or file patterns.
bcipy/main.py
Outdated
| @@ -178,7 +178,7 @@ def execute_task( | |||
| def _clean_up_session( | |||
| display: visual.Window, | |||
| daq: ClientManager, | |||
| servers: List[LslDataServer] = None) -> bool: | |||
| servers: List[LslDataServer] = []) -> bool: | |||
There was a problem hiding this comment.
Probably doesn't matter too much in this context, but this should be a listing error for a dangerous default. https://stackoverflow.com/questions/9526465/best-practice-for-setting-the-default-value-of-a-parameter-thats-supposed-to-be
There was a problem hiding this comment.
I remember reading that, and I think it has bitten us before (I vaguely recall Niklas mentioning it). I am trying to remember why I preferred this here... Probably to avoid the optional!
After the multimodal merge, I need to fix some stuff, so I will revisit this before merging. Thanks for pointing out!
bcipy/language/tests/test_mixture.py
Outdated
| This test requires a valid lm_params.json file and all referenced models""" | ||
| lm = MixtureLanguageModel(response_type=ResponseType.SYMBOL, symbol_set=alphabet()) | ||
| # @pytest.mark.slow | ||
| # def test_default_load(self): |
There was a problem hiding this comment.
Commenting out, I'm not able to get the default image...
Overview
Due to increased contributors and coding projects (bcifit, aiml), I did some cleanup to help with future integrations! This PR,
Ticket
https://www.pivotaltracker.com/story/show/177955157
Contributions
Test
make test-allbcipy --fakeDocumentation
Changelog