Add type hints to test_unit.py test methods (#532)#683
Add type hints to test_unit.py test methods (#532)#683AdityaGupta716 wants to merge 15 commits intoneuroinformatics-unit:mainfrom
Conversation
…euroinformatics-unit#617) Updated keyboard shortcut tooltips in tooltips.py to emphasize CTRL+C (with SHIFT) instead of CTRL+Q for copying filepath to clipboard. CTRL+Q quits the application on macOS, so CTRL+C is the more appropriate shortcut for users on that platform.
for more information, see https://pre-commit.ci
Add module docstring for custom exceptions.
for more information, see https://pre-commit.ci
- Add return type hints (-> None) to test methods in test_unit.py - Specifically: test_datetime_string_replacement and test_process_to_keyword_in_sub_input - Part of issue neuroinformatics-unit#532 effort to extend type hints to test files
tests/tests_unit/test_unit.py
Outdated
| "key", [tags("date"), tags("time"), tags("datetime")] | ||
| ) | ||
| def test_datetime_string_replacement(self, key, underscore_position): | ||
| def test_datetime_string_replacement(self, key, underscore_position): -> None: |
There was a problem hiding this comment.
Syntax error: The return type hint should come before the colon, not after it.
Should be: def test_datetime_string_replacement(self, key, underscore_position) -> None:
Not: def test_datetime_string_replacement(self, key, underscore_position): -> None:
This is causing the pre-commit.ci check to fail with a SyntaxError.
tests/tests_unit/test_unit.py
Outdated
|
|
||
| @pytest.mark.parametrize("prefix", ["sub", "ses"]) | ||
| def test_process_to_keyword_in_sub_input(self, prefix): | ||
| def test_process_to_keyword_in_sub_input(self, prefix): -> None: |
There was a problem hiding this comment.
Same syntax error here. The return type hint should come before the colon.
Should be: def test_process_to_keyword_in_sub_input(self, prefix) -> None:
Not: def test_process_to_keyword_in_sub_input(self, prefix): -> None:
|
@JoeZiminski plz review |
|
Hi @AdityaGupta716 thanks for the reminder, unfortunately things are very busy this week, but I will aim to review this in the second half of next week. Thanks for the contribution! |
Fixed syntax errors in two test methods: 1. test_datetime_string_replacement (line 19): Changed ): -> None: to ) -> None: 2. test_process_to_keyword_in_sub_input (line 48): Changed ): -> None: to ) -> None: The return type hint arrow (->) must come before the colon (:), not after it. This was causing pre-commit.ci checks to fail with SyntaxError.
for more information, see https://pre-commit.ci
|
Hi @AdityaGupta716 thanks for this contribution, the PR contains a few commits from other PRs and so it is difficult to review. Could you please rebase this PR, and #682, onto main and drop the old commits. Please see a guide here. In this way, this PR will only contain changes related to the PR topic. |
|
@JoeZiminski I’ve reverted the unrelated changes so this PR now only updates the type hints in test_unit.py as suggested. |
tests/tests_unit/test_unit.py
Outdated
| ) | ||
| def test_datetime_string_replacement(self, key, underscore_position): | ||
| def test_datetime_string_replacement( | ||
| self, key, underscore_position |
There was a problem hiding this comment.
Thanks @AdityaGupta716, it will be nice to type hint the entire file (including all arguments) as well as the return values
for more information, see https://pre-commit.ci
|
@JoeZiminski Plz review |
|
Hi @AdityaGupta716 similar to 617 this PR will be reviewed / merged after the next release which is coming in a few days, thanks for your patience while we work through the backlog |
2 participants
Description
This PR continues to add type hints to test files as described in issue #532. This PR focuses on adding return type hints to test methods in test_unit.py.
What is this PR?
Why is this PR needed?
Continuing the effort to extend type hints to test files to improve code readability and enable better IDE support.
What does this PR do?
References
Part of #532
How has this PR been tested?