Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

CI has pylint's cyclic-import error disabled. Worth fixing? #16474

Open
DickJC123 opened this issue Oct 14, 2019 · 1 comment
Open

CI has pylint's cyclic-import error disabled. Worth fixing? #16474

DickJC123 opened this issue Oct 14, 2019 · 1 comment

Comments

@DickJC123
Copy link
Contributor

DickJC123 commented Oct 14, 2019

Issue #16401 and resulting PR #16462 exposed the fact by running pylint with the arg --jobs=8, our CI experienced the following behaviors:
- Some classes of errors were not being reported (cyclic-import, duplicate-code, too-many-lines)
- Some errors were being flagged non-deterministically (import-outside-toplevel)

PR #16462 returned the CI's use of pylint to deterministic behavior by setting --jobs=1 and correcting the one import-outside-toplevel error signaled solidly in that mode. The PR also disabled the 3 error classes cyclic-import, duplicate-code, too-many-lines, hopefully pending a more in-depth study.

As asked by @marcoabreu , "Do we want to analyze and fix the cyclic-import errors?"
I hope one of the major python code owners might like to take up this question. The errors can be seen in:

http://jenkins.mxnet-ci.amazon-ml.com/blue/organizations/jenkins/mxnet-validation%2Fsanity/detail/PR-16462/1/pipeline

A PR to address this would start by removing cyclic-import from the list of disabled checks in ./ci/other/pylintrc.

@szha @reminisce @stu1130 @larroy

For Q & A and discussion, please start a discussion thread at https://discuss.mxnet.io

@mxnet-label-bot
Copy link
Contributor

Hey, this is the MXNet Label Bot.
Thank you for submitting the issue! I will try and suggest some labels so that the appropriate MXNet community members can help resolve it.
Here are my recommended label(s): Bug, CI

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants