-
Notifications
You must be signed in to change notification settings - Fork 3.3k
{Pylint} Fix unnecessary-dunder-call #30365
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
Conversation
️✔️AzureCLI-FullTest
|
️✔️AzureCLI-BreakingChangeTest
|
|
Pylint |
| blob, thumbprint = load_cert_file(pfx_file, testpassword) | ||
| except CLIInternalError as e: | ||
| self.assertTrue(e.error_msg.error_msg.__contains__('Invalid password or PKCS12 data')) | ||
| self.assertTrue(e.error_msg.error_msg in 'Invalid password or PKCS12 data') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could use assertIn
| if not is_valid_resource_id(namespace.template_spec): | ||
| raise CLIError('--template-spec is not a valid resource ID.') | ||
| if namespace.template_spec.__contains__("versions") is False: | ||
| if (namespace.template_spec in "versions") is False: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if not ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll check all code later. Some codes are so werid...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These variables are also reversed, it should be "versions" in namespace.template_spec.
Reported to astral-sh/ruff#14423
| raise AssertionError() | ||
| except CLIError as cli_error: | ||
| assert cli_error.__str__() == expected_error | ||
| assert (str(cli_error)) == expected_error |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are nnnecessary parentheses.
Reported to astral-sh/ruff#14597
|
|
||
| # reset session fields, retaining correlation id and application | ||
| _session.__init__(correlation_id=_session.correlation_id, application=_session.application) | ||
| _session.__init__(correlation_id=_session.correlation_id, application=_session.application) # pylint: disable=unnecessary-dunder-call |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It uses __init__ to reset fields, so weird.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@evelyn-ys for awareness.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know why the original author write in this way. Maybe we can try replace with
_session = TelemetrySession(correlation_id=_session.correlation_id, application=_session.application)|
This function contains so many Python black magic, it seems it not used anymore. azure-cli/src/azure-cli/azure/cli/command_modules/network/azure_stack/_util.py Lines 16 to 40 in fa41cb5
|
| setattr(sys.modules[__name__], func_name, delete_func) | ||
| return func_name | ||
|
|
||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Confirmed with necusjz that it can be removed.
PS: It's also removed in this file: https://github.com/Azure/azure-cli/blob/05b75056dc6cfa1b55bb45ea80cbe52d3eb9bb42/src/azure-cli/azure/cli/command_modules/network/_util.py
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would suggest moving this change to a separate PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move to #30453
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
src/azure-cli/azure/cli/command_modules/containerapp/tests/latest/utils.py
Outdated
Show resolved
Hide resolved
|
|
Co-authored-by: Jiashuo Li <[email protected]>
Fix this with
ruff check . --select PLC2801 --fix --preview --unsafe-fixesunnecessary-dunder-callwas disabled in #26685Ref:
https://pylint.readthedocs.io/en/stable/user_guide/messages/convention/unnecessary-dunder-call.html
https://docs.astral.sh/ruff/rules/unnecessary-dunder-call/