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

chore: add tests for gokart parameters #389

Merged
merged 2 commits into from
Aug 1, 2024

Conversation

hiro-o918
Copy link
Contributor

@hiro-o918 hiro-o918 commented Jul 29, 2024

closes: #388

@hiro-o918 hiro-o918 force-pushed the chore/add-tests-gokart-params branch 3 times, most recently from c225dd9 to 534035d Compare July 29, 2024 16:02
@hiro-o918 hiro-o918 force-pushed the chore/add-tests-gokart-params branch from f5b2df0 to c08ba6d Compare August 1, 2024 06:23
@@ -404,9 +406,13 @@ def is_parameter_call(expr: Expression) -> bool:
type_info = callee.node
if type_info is None and isinstance(callee.expr, NameExpr):
return PARAMETER_FULLNAME_MATCHER.match(f'{callee.expr.name}.{callee.name}') is not None
if isinstance(type_info, TypeInfo) and PARAMETER_FULLNAME_MATCHER.match(type_info.fullname):
return True
elif isinstance(callee, NameExpr):
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These conditions are very complicated to review, because these lines are determined by print debugging by tests.
So please check the test cases for this update


# TaskOnKart parameters:
# - `complete_check_at_run`
MyTask(foo=1, bar='bar', baz=False, complete_check_at_run=False)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add complete_check_at_run parameter to check #388 is resolved

MyTask(foo='1')
# TaskOnKart parameters:
# - `complete_check_at_run`
MyTask(foo='1', baz='not bool', complete_check_at_run='not bool')
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can also check invalid type for the attributes of gokart.TaskOnKart

@hiro-o918 hiro-o918 marked this pull request as ready for review August 1, 2024 06:26
gokart/mypy.py Outdated
# the transformation does not affect because the class has `__init__` method of `gokart.TaskOnKart`.
#
# NOTE: `gokart.task.luigi.Task` condition is required for the release of luigi versions without py.typed
if fullname == 'gokart.task.luigi.Task' or fullname == 'luigi.task.Task':
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if fullname == 'gokart.task.luigi.Task' or fullname == 'luigi.task.Task':
if fullname in ['gokart.task.luigi.Task', 'luigi.task.Task']:

@hiro-o918 hiro-o918 force-pushed the chore/add-tests-gokart-params branch from c08ba6d to fcb8be8 Compare August 1, 2024 06:40
@kitagry kitagry merged commit 418f36b into master Aug 1, 2024
5 checks passed
@kitagry kitagry deleted the chore/add-tests-gokart-params branch August 1, 2024 09:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: mypy plugin doesn't collect all gokart.TaskOnKart attributes
2 participants