DM-23173: change W504 to W503 and fix the resulting flake8 warnings #510
Conversation
|
The afw Python unit tests all pass, so I think it is safe to review. However, Jenkins is still running (and I won't merge until it passes): https://ci.lsst.codes/blue/organizations/jenkins/stack-os-matrix/detail/stack-os-matrix/31070/pipeline/ If you'd rather wait for Jenkins to finish, that's fine. |
| @@ -142,8 +142,8 @@ def validate(self): | |||
| return | |||
| if len(self.coeffs) == 1 or self.coeffs[0] != 0 or self.coeffs[1] == 0: | |||
| raise RuntimeError( | |||
| "invalid radial transform coeffs %s: " % (self.coeffs,) + | |||
| "need len(coeffs)=0 or len(coeffs)>1, coeffs[0]==0, " | |||
| "invalid radial transform coeffs %s: " % (self.coeffs,) | |||
There was a problem hiding this comment.
If you changed these lines to f-strings you could remove the + completely.
| afwDisplay.Display(frame=frame).mtv(bkgdImage, title=self._testMethodName + " bkgdImage: " + | ||
| interpStyle.__str__()) | ||
| afwDisplay.Display(frame=frame).mtv(bkgdImage, title=self._testMethodName + " bkgdImage: " | ||
| + interpStyle.__str__()) |
There was a problem hiding this comment.
consider an f-string here to allow the both of the + to go away.
There was a problem hiding this comment.
Note that an f-string does make the line very long (in fact I have to move title=f"..." to the next line). But it squeaks by.
There was a problem hiding this comment.
You can split the f-string over two lines like any other explicit quoted string. You don't need the + to do it though.
| if (foot.getBBox().getMinX() == 0 | ||
| or foot.getBBox().getMaxX() == self.im.getWidth() - 1 | ||
| or foot.getBBox().getMinY() == 0 | ||
| or foot.getBBox().getMaxY() == self.im.getHeight() - 1): |
There was a problem hiding this comment.
This reorganization does look easier to read.
| @@ -870,12 +869,10 @@ def basicTestComputeImageRaise(self, kernel, doRaise, kernelDescr=""): | |||
| try: | |||
| kernel.computeImage(kImage, True) | |||
| if doRaise: | |||
| self.fail(kernelDescr + | |||
| ".computeImage should have raised an exception") | |||
| self.fail(kernelDescr + ".computeImage should have raised an exception") | |||
| except pexExcept.Exception: | ||
| if not doRaise: | ||
| self.fail(kernelDescr + | ||
| ".computeImage should not have raised an exception") | ||
| self.fail(kernelDescr + ".computeImage should not have raised an exception") |
| afwDisplay.Display(frame=1).mtv(im, title=algorithm + | ||
| ": diff image (dx, dy) = (%f, %f)" % (dx, dy)) | ||
| afwDisplay.Display(frame=1).mtv(im, title=algorithm | ||
| + ": diff image (dx, dy) = (%f, %f)" % (dx, dy)) |
|
I will add f-strings where you suggest. I initially did not because it makes the code different than surrounding code and there are over 400 instances of % formatted strings in afw and I didn't want to change them all. |
|
I would like to make the f string changes a separate commit in order to avoid mixing f strings and % formatting in the files. |
No description provided.