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

Update semanal_enum.py #8436

Closed
wants to merge 10 commits into from
Closed

Update semanal_enum.py #8436

wants to merge 10 commits into from

Conversation

Arshaan-256
Copy link

Relaxed the checks for IntEnum arguments.

Relaxed the checks for IntEnum arguments.
@Michael0x2a
Copy link
Collaborator

Thanks for the PR!

It looks like some of the tests are broken -- can you fix them and add a few more? You can find more information on how to run and write tests at https:/python/mypy#tests.

@@ -107,7 +107,8 @@ def parse_enum_call_args(self, call: CallExpr,
if len(args) < 2:
return self.fail_enum_call_arg("Too few arguments for %s()" % class_name, call)
if len(args) > 2:
return self.fail_enum_call_arg("Too many arguments for %s()" % class_name, call)
if call.arg_kinds != [ARG_POS, ARG_POS, ARG_POS]:
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't make much sense in the light of the next if condition: if this one passes the next one (on line 112) will still fail.

Numbers = IntEnum('Numbers', ['ZERO', 'ONE', 'TWO'], start=0)
Numbers = IntEnum('Numbers', ['ZERO', 'ONE', 'TWO'], start=0)
Copy link
Collaborator

@Michael0x2a Michael0x2a left a comment

Choose a reason for hiding this comment

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

Can you add tests for this change? E.g. we should confirm that mypy accepts expressions like Enum('foo', ['a', 'b', 'c'], qualname='bar') but not expressions like Enum('foo', ['a', 'b', 'c'], bad='bar')`, and other similar things.

My previous code review links to some docs on how to run and write tests for mypy.

return self.fail_enum_call_arg("Too many arguments for %s()" % class_name, call)
if call.arg_kinds != [ARG_POS, ARG_POS]:
return self.fail_enum_call_arg("Unexpected arguments to %s()" % class_name, call)
if call.arg_kinds != [ARG_POS, ARG_POS, ARG_NAMED_OPT]:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Enum can accept up to four named arguments, not just one.

You should also try checking call.arg_names to make sure the arguments are what we expect.

Copy link
Author

@Arshaan-256 Arshaan-256 Feb 26, 2020

Choose a reason for hiding this comment

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

@Michael0x2a Sure! Will do it. Can I implement a switch statement to check for named arguments? Could you point to some coding practices that this repo generally follow?

mypy/checkexpr.py Outdated Show resolved Hide resolved
@Arshaan-256 Arshaan-256 closed this Mar 5, 2020
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.

4 participants