-
Notifications
You must be signed in to change notification settings - Fork 6.8k
Disable Pylint false error in numpy_op_signature #16370
Conversation
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.
Why don't you just fix the imports?
Curious to know how this reimport, import and absolute import thingy works on pylint. Stackoverflow search didnt quite give me a good idea! |
fd89e01
to
bd002ab
Compare
@marcoabreu This is a pylint error not a real error of the code. |
bd002ab
to
35b7de6
Compare
Sorry I don't understand. Do you mean this is a bug in pylint? |
@marcoabreu yes based on the following error message
reimport and import are reported at the same line |
As far as I understand, the absolute_import behaviour could already be the default in newer python versions according to https://www.python.org/dev/peps/pep-0328/ Could that mean that pylint reports it as duplicate because it's already done on a system level? What happens if you remove it? |
@marcoabreu Could you explain why it didn't error out for many PRs after Adding |
Could you link a CI run where the pylint failed by any chance? So far, I haven't found an issue where this problem is documented. I'm thinking that it might depend on the underlying Python version being used. Maybe the Python2 pylint says that its fine while the Python3 pylint complains because that's the default behaviour already. If that assumption is true, a conditional import for python 2 only (which we would then deprecate soon) would resolve that. (I understand that it might seem like some hassle for a warning, just trying to understand the background here) |
My PR failed on this pylint check. Let me rebase it and trigger CI again |
Another proof for why |
Okay, sounds good, thanks. But like I thought, it's the python 3 lint that complained. I'll leave it to you discretion, but you might want to consider adding a conditional import which will only do it for python 2. |
@marcoabreu |
Description
(Brief description on what this PR is about)
Checklist
Essentials
Please feel free to remove inapplicable items for your PR.
Changes
Comments