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

Fix Pylint 2.0 violations #2149

Closed
wants to merge 1 commit into from
Closed

Fix Pylint 2.0 violations #2149

wants to merge 1 commit into from

Conversation

netoarmando
Copy link
Member

@netoarmando netoarmando commented Jul 12, 2018

Fix the following violations aiming to support Pylint 2.0

  • unneeded-not (C0113):
    Consider changing "not item in items" to "item not in items" used
    when a boolean expression contains an unneeded negation.

  • useless-import-alias (C0414):
    Import alias does not rename original package Used when an import
    alias is same as original package.e.g using import numpy as numpy
    instead of import numpy as np

  • raising-format-tuple (W0715):
    Exception arguments suggest string formatting might be intended Used
    when passing multiple arguments to an exception constructor, the
    first of them a string literal containing what appears to be
    placeholders intended for formatting

  • bad-continuation (C0330):
    This was already included on the disable list, although with current
    version of pylint (2.0.0.dev2) violations at the end of the files
    are not being ignored.
    See: Global ignore doesn't work at end of file pylint-dev/pylint#2278

  • try-except-raise (E0705):
    The except handler raises immediately Used when an except handler
    uses raise as its first or only operator. This is useless because it
    raises back the exception immediately. Remove the raise operator or
    the entire try-except-raise block!

  • consider-using-set-comprehension (R1718):
    Consider using a set comprehension Although there is nothing
    syntactically wrong with this code, it is hard to read and can be
    simplified to a set comprehension.Also it is faster since you don't
    need to create another transient list

  • dict-keys-not-iterating (W1655):
    dict.keys referenced when not iterating Used when dict.keys is
    referenced in a non-iterating context (returns an iterator in
    Python 3)

  • comprehension-escape (W1662):
    Using a variable that was bound inside a comprehension Emitted when
    using a variable, that was bound in a comprehension handler, outside
    of the comprehension itself. On Python 3 these variables will be
    deleted outside of the comprehension.

Issue: https://pagure.io/freeipa/issue/7614

Signed-off-by: Armando Neto abiagion@redhat.com

@netoarmando
Copy link
Member Author

Travis is failing because its Pylint version doesn't recognize the new violations.

@tiran
Copy link
Member

tiran commented Jul 13, 2018

Unfortunately you have to catch bad-option-value while we support pylint 1.x.

Copy link
Member

@tiran tiran left a comment

Choose a reason for hiding this comment

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

You have to keep the scope of pylint rules in mind. On some occasions you disabled rules in a broader scope. https://pylint.readthedocs.io/en/latest/user_guide/message-control.html contains several examples. It's sometimes annoying, but you'll get used to it.

By the way, I like your verbose commit messages!

@@ -139,6 +139,7 @@ def execute(self, _name, *args, **options):
if _name not in self.Command:
raise CommandError(name=_name)
return self.Command[_name](*args, **options)
# pylint: disable=try-except-raise
Copy link
Member

Choose a reason for hiding this comment

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

If you put it there, then it will apply to the rest of the block. You have to put it next to the except statement, so it only applies to the except ...: block:

except PublicError:  # pylint: disable=try-except-raise, bad-option-value
    raise

or if the comment doesn't fit within the line length limit:

# pylint: disable=try-except-raise, bad-option-value
except PublicError:
    raise
# pylint: enable=try-except-raise, bad-option-value

Copy link
Member Author

Choose a reason for hiding this comment

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

I wasn't able to make this work, with pylint 1.9.5, it complains the same way, only adding bad-option-value to pylintrc changes the result. Submitting the changes anyway to see if travis acts the same (unexpected) way.

@@ -1071,6 +1071,7 @@ def create_connection(self, ccache=None, verbose=None, fallback=None,
)
# We don't care about the response, just that we got one
return serverproxy
# pylint: disable=try-except-raise
Copy link
Member

Choose a reason for hiding this comment

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

same here

@tiran tiran added ipa-4.7 ipa-4-6 Mark for backport to ipa 4.6 labels Jul 13, 2018
Fix the following violations aiming to support Pylint 2.0

- `unneeded-not` (C0113):
  Consider changing "not item in items" to "item not in items" used
  when a boolean expression contains an unneeded negation.

- `useless-import-alias` (C0414):
  Import alias does not rename original package Used when an import
  alias is same as original package.e.g using import numpy as numpy
  instead of import numpy as np

- `raising-format-tuple` (W0715):
  Exception arguments suggest string formatting might be intended Used
  when passing multiple arguments to an exception constructor, the
  first of them a string literal containing what appears to be
  placeholders intended for formatting

- `bad-continuation` (C0330):
  This was already included on the disable list, although with current
  version of pylint (2.0.0.dev2) violations at the end of the files
  are not being ignored.
  See: pylint-dev/pylint#2278

- `try-except-raise` (E0705):
  The except handler raises immediately Used when an except handler
  uses raise as its first or only operator. This is useless because it
  raises back the exception immediately. Remove the raise operator or
  the entire try-except-raise block!

- `consider-using-set-comprehension` (R1718):
  Consider using a set comprehension Although there is nothing
  syntactically wrong with this code, it is hard to read and can be
  simplified to a set comprehension.Also it is faster since you don't
  need to create another transient list

- `dict-keys-not-iterating` (W1655):
  dict.keys referenced when not iterating Used when dict.keys is
  referenced in a non-iterating context (returns an iterator in
  Python 3)

- `comprehension-escape` (W1662):
  Using a variable that was bound inside a comprehension Emitted when
  using a variable, that was bound in a comprehension handler, outside
  of the comprehension itself. On Python 3 these variables will be
  deleted outside of the comprehension.

Issue: https://pagure.io/freeipa/issue/7614

Signed-off-by: Armando Neto <abiagion@redhat.com>
@tiran tiran added ack Pull Request approved, can be merged pushed Pull Request has already been pushed labels Jul 14, 2018
@tiran
Copy link
Member

tiran commented Jul 14, 2018

master:

  • d135719 Fix Pylint 2.0 violations

@tiran tiran closed this Jul 14, 2018
@tiran tiran removed the ipa-4.7 label Jul 14, 2018
@netoarmando netoarmando deleted the issue-7614-fix-pylint2-violations branch July 14, 2018 22:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ack Pull Request approved, can be merged ipa-4-6 Mark for backport to ipa 4.6 pushed Pull Request has already been pushed
Projects
None yet
2 participants