Treatment of linter and coverage flags #1214

Open
gedeck opened this Issue Dec 17, 2016 · 0 comments

Projects

None yet

1 participant

@gedeck
Contributor
gedeck commented Dec 17, 2016

This is a follow up to a conversation on a PR on how to deal with tool specific flags. The following process was suggested:

Let me propose the following. There are a couple of pydev specific warnings:

  • @UnresolvedImport: They are always associated with rdkit.six.moves and I will remove these. In principle, pydev allows to suppress this warning. However, it seems this currently doesn't work.
  • @UnusedWildImport: Where possible, I changed that to specific imports. There is only one in rdkit.ML.Descriptors.Parser and here it is necessary (?) as the code uses eval
  • @ReservedAssignment: There are only few in the code and they highlight cases where we have reserved words like min or max as arguments of functions. I didn't change the argument names as callers may use keywords arguments. I would prefer leaving them in as a warning.
  • @UnusedVariable: Cases where a tuple is unpacked and some parts of the tuple are unused. I prefer keeping unused variables to a minimum. One option is to replace the name with an _. The other option (probably better) is to use namedtuples instead of returning a large number of variables. We could consider moving the code to use namedtuples more. In this case, I would prefer leaving this in for now as an indication where to find places to change. What do you think?
  • @IgnorePep8: I'll remove that.

Then there are 31 pylint warnings. I can remove them.

Finally, pragma: nocover. They are flags for blocks of code that are not tested, but where the tests are not important (e.g. verbose output, the unittest.run() code in Unittest files). I prefer to leave these in. They are a useful as they indicate a conscious decision.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment