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

Bump pylint and flake8 #232

Merged
merged 5 commits into from
Feb 2, 2021
Merged

Bump pylint and flake8 #232

merged 5 commits into from
Feb 2, 2021

Conversation

jiasli
Copy link
Member

@jiasli jiasli commented Jan 26, 2021

Bump pylint and flake8 to the latest version to make sure the code comply with the latest Python 3 standards.

@@ -83,7 +83,7 @@ def __getattr__(self, name):

def __setattr__(self, name, value): # pylint: disable=inconsistent-return-statements
if name == 'type':
return super(CLICommandArgument, self).__setattr__(name, value)

Choose a reason for hiding this comment

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

It's a big change for our code. Can you provide more information about this.

Copy link
Member Author

@jiasli jiasli Jan 29, 2021

Choose a reason for hiding this comment

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

If we run pylint==2.6.0 on the current code, it errors out:

> tox -e py38

************* Module knack.arguments
knack\arguments.py:86:19: R1725: Consider using Python 3 style super() without arguments (super-with-arguments)
knack\arguments.py:189:24: R1725: Consider using Python 3 style super() without arguments (super-with-arguments)
knack\arguments.py:212:24: R1725: Consider using Python 3 style super() without arguments (super-with-arguments)
knack\arguments.py:246:24: R1725: Consider using Python 3 style super() without arguments (super-with-arguments)
knack\arguments.py:297:24: R1725: Consider using Python 3 style super() without arguments (super-with-arguments)
************* Module knack.commands
knack\commands.py:283:12: W0707: Consider explicitly re-raising using the 'from' keyword (raise-missing-from)
************* Module knack.deprecation
knack\deprecation.py:89:8: R1725: Consider using Python 3 style super() without arguments (super-with-arguments)
knack\deprecation.py:141:8: R1725: Consider using Python 3 style super() without arguments (super-with-arguments)
************* Module knack.experimental
knack\experimental.py:55:8: R1725: Consider using Python 3 style super() without arguments (super-with-arguments)
knack\experimental.py:76:8: R1725: Consider using Python 3 style super() without arguments (super-with-arguments)
************* Module knack.help
knack\help.py:95:8: R1725: Consider using Python 3 style super() without arguments (super-with-arguments)
knack\help.py:126:8: R1725: Consider using Python 3 style super() without arguments (super-with-arguments)
knack\help.py:245:8: R1725: Consider using Python 3 style super() without arguments (super-with-arguments)
knack\help.py:269:8: R1725: Consider using Python 3 style super() without arguments (super-with-arguments)
knack\help.py:328:8: R1725: Consider using Python 3 style super() without arguments (super-with-arguments)
knack\help.py:348:8: R1725: Consider using Python 3 style super() without arguments (super-with-arguments)
************* Module knack.output
knack\output.py:81:8: W0707: Consider explicitly re-raising using the 'from' keyword (raise-missing-from)
************* Module knack.parser
knack\parser.py:112:8: R1725: Consider using Python 3 style super() without arguments (super-with-arguments)
knack\parser.py:226:15: R1725: Consider using Python 3 style super() without arguments (super-with-arguments)
knack\parser.py:262:15: R1725: Consider using Python 3 style super() without arguments (super-with-arguments)
************* Module knack.preview
knack\preview.py:55:8: R1725: Consider using Python 3 style super() without arguments (super-with-arguments)
knack\preview.py:76:8: R1725: Consider using Python 3 style super() without arguments (super-with-arguments)
************* Module knack.query
knack\query.py:27:12: W0707: Consider explicitly re-raising using the 'from' keyword (raise-missing-from)
************* Module knack.util
knack\util.py:45:8: R1725: Consider using Python 3 style super() without arguments (super-with-arguments)
************* Module knack.testsdk.base
knack\testsdk\base.py:29:8: R1725: Consider using Python 3 style super() without arguments (super-with-arguments)
knack\testsdk\base.py:85:8: R1725: Consider using Python 3 style super() without arguments (super-with-arguments)
knack\testsdk\base.py:116:8: R1725: Consider using Python 3 style super() without arguments (super-with-arguments)
knack\testsdk\base.py:254:12: W0707: Consider explicitly re-raising using the 'from' keyword (raise-missing-from)
************* Module knack.testsdk.checkers
knack\testsdk\checkers.py:67:12: W0707: Consider explicitly re-raising using the 'from' keyword (raise-missing-from)
knack\testsdk\checkers.py:80:12: W0707: Consider explicitly re-raising using the 'from' keyword (raise-missing-from)
knack\testsdk\checkers.py:93:12: W0707: Consider explicitly re-raising using the 'from' keyword (raise-missing-from)
************* Module knack.testsdk.exceptions
knack\testsdk\exceptions.py:10:8: R1725: Consider using Python 3 style super() without arguments (super-with-arguments)
knack\testsdk\exceptions.py:17:8: R1725: Consider using Python 3 style super() without arguments (super-with-arguments)
knack\testsdk\exceptions.py:25:8: R1725: Consider using Python 3 style super() without arguments (super-with-arguments)

-------------------------------------------------------------------
Your code has been rated at 9.87/10 (previous run: 10.00/10, -0.13)

The help message is pretty straight-forward:

> pylint --help-msg R1725,W0707
:super-with-arguments (R1725): *Consider using Python 3 style super() without arguments*
  Emitted when calling the super() builtin with the current class and instance.
  On Python 3 these arguments are the default and they can be omitted. This
  message belongs to the refactoring checker.

:raise-missing-from (W0707): *Consider explicitly re-raising using the 'from' keyword*
  Python 3's exception chaining means it shows the traceback of the current
  exception, but also the original exception. Not using `raise from` makes the
  traceback inaccurate, because the message implies there is a bug in the
  exception-handling code itself, which is a separate situation than wrapping
  an exception. This message belongs to the exceptions checker.

Choose a reason for hiding this comment

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

As I know, python2 does not support this, and pycharm are using the old format in code autocompletion.

Copy link
Member Author

Choose a reason for hiding this comment

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

We have dropped Python 2 support long time ago (#174). PyCharm is only an IDE and is not the standard. We had better follow PEP 3135 instead.

Choose a reason for hiding this comment

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

We need to make the same changes for azure-cli and azure-cli-extensions after using the new knack version with this bump?

Copy link
Member Author

Choose a reason for hiding this comment

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

Definitely not. Knack itself doesn't require pylint and flake8. This PR only makes sure Knack complies with the latest pylint and flake8.

Choose a reason for hiding this comment

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

I see. pylint is still pinned to 2.3.0 in azdev.

Comment on lines +281 to +282
except (ValueError, AttributeError) as ex:
raise ValueError("The operation '{}' is invalid.".format(operation)) from ex
Copy link
Member Author

Choose a reason for hiding this comment

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

Use Python's built-in Exception Chaining as recommended by W0707:

raise-missing-from (W0707):
Consider explicitly re-raising using the 'from' keyword Python 3's exception chaining means it shows the traceback of the current exception, but also the original exception. Not using raise from makes the traceback inaccurate, because the message implies there is a bug in the exception-handling code itself, which is a separate situation than wrapping an exception.

@jiasli jiasli merged commit cd59031 into microsoft:master Feb 2, 2021
@jiasli jiasli deleted the pylint branch February 2, 2021 13:14
@jiasli jiasli mentioned this pull request Feb 2, 2021
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.

3 participants