Overhaul noqa directives by EliahKagan · Pull Request #1760 · gitpython-developers/GitPython

@EliahKagan

- Remove some noqa directives that are not needed at all anymore.
  But there may still be some others I did not find.

- Fix the code and remove the noqa direcive in places where the
  change is very simple and does not affect the interface (e.g.,
  what could be taken to be public or not, what symbols are
  available and what they refer to, etc.), the change makes the
  code more idiomatic or clearer, and it fixes the flake8 complaint
  completely.

- Turn file-level noqa directives into statement-level ones. A noqa
  directive at file level, even if it lists one or more specific
  error/warning numbers, actually suppresses flake8 for the entire
  file. So the main benefit here is to enable other warnings. But
  this also makes it clear what is producing the ones we suppress,
  which makes it easier to know what is intentional and how it may
  be reasonable to change it in the future.

- Move noqa directives at the end of multi-line imports to the top
  line, where flake8 honors them, and eliminate redundant
  directives that were added to work around their ineffectiveness.
  (This is not really separate from the above point, since this is
  often what allowed file-level directives to be removed.)

- Specify the errors/warnings each "noqa" should suppress. That is,
  this turns every bare "noqa", aside from those eliminated as
  described above, into a "noqa" for one or more specific
  errors/warnings. This makes the intent clearer and avoids
  oversuppression.

- Add missing ":" between "noqa" and error/warning numbers. When
  these were absent, the intention was to suppress only the
  specifically listed errors/warnings, but the effect was that the
  listed numbers were ignored and *everything* was suppressed. In a
  couple of cases it was necessary to change the listed numbers to
  correct the list to what actually needed to be suppressed.

- Write every "noqa" in lower-case (not "NOQA"). There did not
  appear to be a systematic reason for the different casing, and
  having them all cased the same makes it easier to avoid mistakes
  when grepping for them.

- Where an import is unused and does not appear intended to be
  accesed from other modules, and is present only to support code
  in the same module that is commented out, but whose removal
  should be considered separately, comment out the statement or
  part of the statement that imports it as well, rather than
  writing a "noqa" for the unused import. This applies to only one
  file, git/objects/util.py.