PEP8 compliance: review (& justify chosen) exclusions

[sadielbartholomew]

For cfdm as for cf-python defined in an equivalent issue NCAS-CMS/cf-python#83; the cfdm codebase is now PEP8-compliant under the interpretation & scope of the pycodestyle library, with the exception of several rules I have explicitly excluded (where there is no easy way to exclude instead on a per-case/line basis, sadly). We should review these exclusions, in this case being:

cfdm/cfdm/test/test_style.py

Lines 22 to 31 in 7319bf8

	# These are pycodestyle errors and warnings to explicitly ignore. For
	# descriptions for each code see:
	# https://pep8.readthedocs.io/en/latest/intro.html#error-codes
	pep8_check.options.ignore += ( # ignored because...
	'W605', # ...false positives on regex and LaTeX expressions
	'E272', # ...>1 spaces to align keywords in long import listings
	'E402', # ...justified lower module imports in {.., core}/__init__
	'E501', # ...docstring examples include output lines >79 chars
	'E722', # ...lots of "bare except" cases need to be addressed
	)

& decide whether pycodestyle is the right tool, among many options, for our requirements on linting.

[davidhassell]

I think that I'm happy with all of these exclusions, apart from

'E722',  # ...lots of "bare except" cases need to be addressed 

We should, at the very least, turn these into except Exception:. I'll do that, for now, so that the E722 can be removed from the list.

[sadielbartholomew]

We should, at the very least, turn these into except Exception

Ah, good idea! I would have done that when finalising the PEP8 amendments but I assumed that except Exception would also raise that error code, but having tried it now I see it doesn't. Interestingly, I see except BaseException: does not raise that pycodestyle error code even though it is logically equivalent to except:! So it seems they just want people to be specific but very generic exceptions are fine.

On the topic, I have noticed that cfdm and cf-python do not use any custom exceptions, with the exception (hah) of a pair of DeprecationErrors and UMFileException (as far as I can see by grep). Was that deliberate? I'm thinking that, without going crazy with it, we could likely make some classes a bit cleaner & improve user feedback on errors etc. by creating & applying some custom exception classes for which to delegate some error handling. I can look into that separately (after I have tied up current loose ends & more), assuming you agree it would be beneficial.

[davidhassell]

Hi Sadie - good. I have made those changes and will push them shortly.

Custom exceptions are absolutely fine (there's no particular reason why we don't have many). Let's close this issue and pick up making custom exceptions in a new thread.

[sadielbartholomew]

Good plan.

I have made those changes and will push them shortly.

FYI I quickly added in PEP8 checks on some Python files outside of the cfdm/ directory but I made sure there were no bare exceptions on any of those as part of PEP8-ing them. It shouldn't influence what you have done, but just to tell you in case you had seen those commits & were wondering.