Fixes and PEP 8/pyflakes updates for utils #78
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The main changes are to correctly catch errors, although it is not clear if the OutOfBoundErr class is ever raised (perhaps from compiled code?). There are several other changes: removal of unused variables, chamge min/max variable names (not all functions are changed), comments on screen output that maybe should use the logging output, more Pythonic boolean checks (e.g. change 'False == xxx' to 'not xxx'), and a couple of PEP8 adding-space-around-symbol changes that accidentally made it in.
These are mainly for Sphinx (change : to :: to introduce a section of text). Several PEP8 changes (removal of spaces around symbols) have crept in with this commit.
PEP-8 related changes: add blank lines before top-level symbols so there are two such lines separating definitions; ensure there's a space after the comment character; ensure comment line doesn't make the line too long.
PEP-8 changes to syntax, not functionality (in the most part; due to the desire to keep line-lengths short, some lines have been split up and the code slightly-rewritten to account for this).
A PEP-8 suggestion: it showed that gsl_fcmp is not exported by this module which is probably intentional.
|
Looks good to me. |
Merged
|
@wmclaugh (as you were assigned to it). I reviewed this PR and I think it should be merged. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR replaces #70.
Release Note
OutOfBoundErr exceptions are properly caught in some sherpa.utils functions are properly caught.
There were several places where screen output used either print or sys.stderr.write: these have been changed to use the debug and warning loggers instead (respectively). Several routines which use min and max as variable names have been changed. A number of other pyflakes/PEP-8 style violations were addressed, e.g. a more Pythonic use of boolean variables (e.g. not xxx rather than False == xxx).
Description
These commits fix a potential error, in that
OutOfBoundErrexceptions were not being caught (although it's not actually clear that this is ever thrown). There were several places where screen output used eitherprintorsys.stderr.write: these have been changed to use thedebugandwarningloggers instead (respectively). Several routines which useminandmaxas variable names have been changed. There are very-minor documentation changes for Sphinx (mainly change:to::when introducing some maths).It also addresses a number of pyflakes/PEP-8 violations: more Pythonic use of boolean variables (e.g.
not xxxrather thanFalse == xxx); add spaces between operators; remove spaces before/after various brackets; line length; two blank lines between top-level declarations; explicit module export; remove blank characters at the ends of lines.The commits have been broken up into separate sections to try and make reviewing the changes somewhat easier.
The reason for these changes is that I want to make some changes to the test class code, but found a number of issues that were being flagged up by
so decided to clean these up first.