Disable Pylint false error in numpy_op_signature

[stu1130]

Description

(Brief description on what this PR is about)

Checklist

Essentials

Please feel free to remove inapplicable items for your PR.

• The PR title starts with [MXNET-$JIRA_ID], where $JIRA_ID refers to the relevant JIRA issue created (except PRs with tiny changes)
• Changes are complete (i.e. I finished coding on this PR)
• All changes have test coverage:
• Unit tests are added for small changes to verify correctness (e.g. adding a new operator)
• Nightly tests are added for complicated/long-running ones (e.g. changing distributed kvstore)
• Build tests will be added for build configuration changes (e.g. adding a new build option with NCCL)
• Code is well-documented:
• For user-facing API changes, API doc string has been updated.
• For new C++ functions in header files, their functionalities and arguments are documented.
• For new examples, README.md is added to explain the what the example does, the source of the dataset, expected performance on test set and reference to the original paper if applicable
• Check the API doc at https://mxnet-ci-doc.s3-accelerate.dualstack.amazonaws.com/PR-$PR_ID/$BUILD_ID/index.html
• To the my best knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

Changes

• Feature1, tests, (and when applicable, API doc)
• Feature2, tests, (and when applicable, API doc)

Comments

• If this change is a backward incompatible change, why must this change be made.
• Interesting edge cases to note here

[marcoabreu]

Why don't you just fix the imports?

[ChaiBapchya]

Curious to know how this reimport, import and absolute import thingy works on pylint. Stackoverflow search didnt quite give me a good idea!

[reminisce]

@marcoabreu This is a pylint error not a real error of the code.

[marcoabreu]

Sorry I don't understand. Do you mean this is a bug in pylint?

[stu1130]

@marcoabreu yes based on the following error message

python/mxnet/numpy_op_signature.py:20:0: W0404: Reimport 'absolute_import' (imported line 20) (reimported)

reimport and import are reported at the same line

[marcoabreu]

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?

[reminisce]

@marcoabreu Could you explain why it didn't error out for many PRs after numpy_op_signature.py was introduced? The pylint behavior is flaky at least.

Adding absolute_import explicitly enforces consistent importing behavior between python2 and python3 throughout the current file without relying on the assumption that this is done somewhere else. So I don't feel comfortable about deleting it to just make pylint happy.

[marcoabreu]

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)

[stu1130]

My PR failed on this pylint check. Let me rebase it and trigger CI again

[reminisce]

Another proof for why absolute_import is required in python2. If that were deleted, and you typed import numpy in that file which was expected to import the official numpy package, however, it would just give you mxnet.numpy because it did not look up in the top-level packages.

[marcoabreu]

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.

[reminisce]

@marcoabreu from __future__ import absolute_import is used almost everywhere throughout the code base. And please read the error message dumped by pylint; you will find that import and reimport are reported at the same line. Obviously, pylint made a mistake in this particular file. Adding a conditional import for working around pylint's mistake does not sound like desirable practice.