Skip to content

Commit

Permalink
STYLE: Update python code with statement fixes
Browse files Browse the repository at this point in the history
This fixes many of the pycodestyle E7 statement error codes.

Bulk of updates performed using autopep8 CLI
with the following command:

  autopep8 --in-place --select E701,E703,E712,E713,E714,E741 $(git ls-files '*.py')

See https://github.com/hhatto/autopep8#readme

References:

* Multiple statements on one line (colon)
  See https://www.flake8rules.com/rules/E701.html

* Statement ends with a semicolon
  See https://www.flake8rules.com/rules/E703.html

* Comparison to true should be 'if cond is true:' or 'if cond:'
  See https://www.flake8rules.com/rules/E712.html

* Test for membership should be 'not in'
  See https://www.flake8rules.com/rules/E713.html

* Test for object identity should be 'is not'
  See https://www.flake8rules.com/rules/E714.html

* Do not use variables named 'I', 'O', or 'l'
  See https://www.flake8rules.com/rules/E741.html
  • Loading branch information
jamesobutler authored and lassoan committed Jun 22, 2022
1 parent c48a846 commit 34696fb
Show file tree
Hide file tree
Showing 24 changed files with 83 additions and 74 deletions.
12 changes: 0 additions & 12 deletions .flake8
Original file line number Diff line number Diff line change
Expand Up @@ -46,22 +46,10 @@ ignore = \
E402, \
# the backslash is redundant between bracket
E502, \
# multiple statements on one line (colon)
E701, \
# statement ends with a semicolon
E703, \
# comparison to True should be 'if cond is True:' or 'if cond:'
E712, \
# test for membership should be 'not in'
E713, \
# test for object identity should be 'is not'
E714, \
# do not use bare 'except'
E722, \
# do not assign a lambda expression, use a def
E731, \
# ambiguous variable name 'l'
E741, \
# 'from module import *' used; unable to detect undefined names
F403, \
# Name may be undefined, or defined from star import
Expand Down
30 changes: 20 additions & 10 deletions Applications/SlicerApp/Testing/Python/MeasureStartupTimes.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,10 @@ def collect_startup_times_normal(output_file, drop_cache=False, display_output=F
(duration, result) = runSlicerAndExitWithTime(slicer_executable, test, drop_cache=drop_cache)
(returnCode, stdout, stderr) = result
if display_output:
if stdout: print("STDOUT [%s]\n" % stdout)
if stderr and returnCode == EXIT_SUCCESS: print("STDERR [%s]\n" % stderr)
if stdout:
print("STDOUT [%s]\n" % stdout)
if stderr and returnCode == EXIT_SUCCESS:
print("STDERR [%s]\n" % stderr)
results[" ".join(test)] = duration
with open(output_file, 'w') as file:
file.write(json.dumps(results, indent=4))
Expand Down Expand Up @@ -84,8 +86,10 @@ def collect_startup_times_overall(output_file, drop_cache=False, display_output=

(returnCode, stdout, stderr) = result
if display_output:
if stdout: print("STDOUT [%s]\n" % stdout)
if stderr and returnCode == EXIT_SUCCESS: print("STDERR [%s]\n" % stderr)
if stdout:
print("STDOUT [%s]\n" % stdout)
if stderr and returnCode == EXIT_SUCCESS:
print("STDERR [%s]\n" % stderr)

with open(output_file, 'w') as file:
file.write(json.dumps(results, indent=4))
Expand Down Expand Up @@ -155,8 +159,10 @@ def collect_startup_times_including_one_module(output_file, module_list, drop_ca
(duration, result) = runSlicerAndExitWithTime(slicer_executable, test, drop_cache=drop_cache)
(returnCode, stdout, stderr) = result
if display_output:
if stdout: print("STDOUT [%s]\n" % stdout)
if stderr and returnCode == EXIT_SUCCESS: print("STDERR [%s]\n" % stderr)
if stdout:
print("STDOUT [%s]\n" % stdout)
if stderr and returnCode == EXIT_SUCCESS:
print("STDERR [%s]\n" % stderr)
if returnCode != EXIT_SUCCESS:
# XXX Ignore module with dependencies
duration = None
Expand All @@ -180,8 +186,10 @@ def collect_startup_times_excluding_one_module(output_file, module_list, drop_ca
(duration, result) = runSlicerAndExitWithTime(slicer_executable, ['--testing', '--modules-to-ignore', moduleName], drop_cache=drop_cache)
(returnCode, stdout, stderr) = result
if display_output:
if stdout: print("STDOUT [%s]\n" % stdout)
if stderr and returnCode == EXIT_SUCCESS: print("STDERR [%s]\n" % stderr)
if stdout:
print("STDOUT [%s]\n" % stdout)
if stderr and returnCode == EXIT_SUCCESS:
print("STDERR [%s]\n" % stderr)
if returnCode != EXIT_SUCCESS:
# XXX Ignore module with dependencies
duration = None
Expand All @@ -204,8 +212,10 @@ def collect_startup_times_modules_to_load(output_file, modules_to_load, module_l
(duration, result) = runSlicerAndExitWithTime(slicer_executable, test, drop_cache=drop_cache)
(returnCode, stdout, stderr) = result
if display_output:
if stdout: print("STDOUT [%s]\n" % stdout)
if stderr and returnCode == EXIT_SUCCESS: print("STDERR [%s]\n" % stderr)
if stdout:
print("STDOUT [%s]\n" % stdout)
if stderr and returnCode == EXIT_SUCCESS:
print("STDERR [%s]\n" % stderr)

results = {}
results[" ".join(modulesToIgnore)] = duration
Expand Down
4 changes: 2 additions & 2 deletions Applications/SlicerApp/Testing/Python/SlicerBoundsTest.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ def setUp(self):
slicer.mrmlScene.Clear(0)

def assertListAlmostEquals(self, list, expected):
for l, e in zip(list, expected):
self.assertAlmostEqual(l, e)
for list_item, expected_item in zip(list, expected):
self.assertAlmostEqual(list_item, expected_item)

def runTest(self):
"""Run as few or as many tests as needed here.
Expand Down
3 changes: 2 additions & 1 deletion Base/Python/slicer/slicerqt.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@
# that way the following try/except could be avoided.
try:
import slicer.cli
except: pass
except:
pass


# -----------------------------------------------------------------------------
Expand Down
5 changes: 3 additions & 2 deletions Base/Python/slicer/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,8 @@ def lookupTopLevelWidget(objectName):
from slicer import app
for w in app.topLevelWidgets():
if hasattr(w, 'objectName'):
if w.objectName == objectName: return w
if w.objectName == objectName:
return w
# not found
raise RuntimeError("Failed to obtain reference to '%s'" % objectName)

Expand Down Expand Up @@ -1259,7 +1260,7 @@ def reloadScriptedModule(moduleName):
filePath = modulePath(moduleName)
p = os.path.dirname(filePath)

if not p in sys.path:
if p not in sys.path:
sys.path.insert(0, p)

with open(filePath, encoding='utf8') as fp:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,7 @@ def test_MarkupsInViewsSelfTest1(self):
logic = MarkupsInViewsSelfTestLogic()
retval = logic.run()

if retval == True:
if retval is True:
self.delayDisplay('Test passed!')
else:
self.delayDisplay('Test failed!')
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,8 @@ def addPoint(self, ras):

# Don't allow adding points on except on the active slice
# (where first point was laid down)
if self.activeSliceOffset != currentSliceOffset: return
if self.activeSliceOffset != currentSliceOffset:
return

# Keep track of node state (in case of pan/zoom)
sliceNode = sliceLogic.GetSliceNode()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ def section_SetupPathsAndNames(self):
self.cloneNodeNamePostfix = slicer.qSlicerSubjectHierarchyCloneNodePlugin().getCloneNodeNamePostfix()

# Test printing of all context menu actions and their section numbers
pluginHandler = slicer.qSlicerSubjectHierarchyPluginHandler().instance();
pluginHandler = slicer.qSlicerSubjectHierarchyPluginHandler().instance()
print(pluginHandler.dumpContextMenuActions())

# ------------------------------------------------------------------------------
Expand Down
4 changes: 2 additions & 2 deletions Modules/Scripted/DICOM/DICOM.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,10 +125,10 @@ def onURLReceived(self, urlString):
for key, value in query.queryItems(qt.QUrl.FullyDecoded):
queryMap[key] = qt.QUrl.fromPercentEncoding(value)

if not "dicomweb_endpoint" in queryMap:
if "dicomweb_endpoint" not in queryMap:
logging.debug("DICOM module ignores URL without dicomweb_endpoint query parameter: " + urlString)
return
if not "studyUID" in queryMap:
if "studyUID" not in queryMap:
logging.debug("DICOM module ignores URL without studyUID query parameter: " + urlString)
return

Expand Down
12 changes: 6 additions & 6 deletions Modules/Scripted/DICOMLib/DICOMBrowser.py
Original file line number Diff line number Diff line change
Expand Up @@ -217,15 +217,15 @@ def promptForExtensions(self):
displayedTypeDescriptions = []
for extension in extensionsToOffer:
typeDescription = extension['typeDescription']
if not typeDescription in displayedTypeDescriptions:
if typeDescription not in displayedTypeDescriptions:
# only display each data type only once
message += ' ' + typeDescription + '\n'
displayedTypeDescriptions.append(typeDescription)
message += "\nThe following extension%s not installed, but may help you work with this data:\n\n" % pluralOrNot
displayedExtensionNames = []
for extension in extensionsToOffer:
extensionName = extension['name']
if not extensionName in displayedExtensionNames:
if extensionName not in displayedExtensionNames:
# only display each extension name only once
message += ' ' + extensionName + '\n'
displayedExtensionNames.append(extensionName)
Expand Down Expand Up @@ -488,7 +488,7 @@ def loadCheckedLoadables(self):
def showReferenceDialogAndProceed(self):
referencesDialog = DICOMReferencesDialog(self, loadables=self.referencedLoadables)
answer = referencesDialog.exec_()
if referencesDialog.rememberChoiceAndStopAskingCheckbox.checked == True:
if referencesDialog.rememberChoiceAndStopAskingCheckbox.checked is True:
if answer == qt.QMessageBox.Yes:
qt.QSettings().setValue('DICOM/automaticallyLoadReferences', qt.QMessageBox.Yes)
if answer == qt.QMessageBox.No:
Expand All @@ -497,7 +497,7 @@ def showReferenceDialogAndProceed(self):
# each check box corresponds to a referenced loadable that was selected by examine;
# if the user confirmed that reference should be loaded, add it to the self.loadablesByPlugin dictionary
for plugin in self.referencedLoadables:
for loadable in [l for l in self.referencedLoadables[plugin] if l.selected]:
for loadable in [loadable_item for loadable_item in self.referencedLoadables[plugin] if loadable_item.selected]:
if referencesDialog.checkboxes[loadable].checked:
self.loadablesByPlugin[plugin].append(loadable)
self.loadablesByPlugin[plugin] = list(set(self.loadablesByPlugin[plugin]))
Expand All @@ -507,7 +507,7 @@ def showReferenceDialogAndProceed(self):

def addReferencesAndProceed(self):
for plugin in self.referencedLoadables:
for loadable in [l for l in self.referencedLoadables[plugin] if l.selected]:
for loadable in [loadable_item for loadable_item in self.referencedLoadables[plugin] if loadable_item.selected]:
self.loadablesByPlugin[plugin].append(loadable)
self.loadablesByPlugin[plugin] = list(set(self.loadablesByPlugin[plugin]))
self.proceedWithReferencedLoadablesSelection()
Expand Down Expand Up @@ -615,7 +615,7 @@ def _addLoadableCheckboxes(self):
self.checkBoxGroupBox = qt.QGroupBox("References")
self.checkBoxGroupBox.setLayout(qt.QFormLayout())
for plugin in self.loadables:
for loadable in [l for l in self.loadables[plugin] if l.selected]:
for loadable in [loadable_item for loadable_item in self.loadables[plugin] if loadable_item.selected]:
checkBoxText = loadable.name + ' (' + plugin.loadType + ') '
cb = qt.QCheckBox(checkBoxText, self)
cb.checked = True
Expand Down
4 changes: 2 additions & 2 deletions Modules/Scripted/DICOMLib/DICOMUtils.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ def loadPatientByUID(patientUID):
raise OSError('DICOM module or database cannot be accessed')

patientUIDstr = str(patientUID)
if not patientUIDstr in slicer.dicomDatabase.patients():
if patientUIDstr not in slicer.dicomDatabase.patients():
raise OSError('No patient found with DICOM database UID %s' % patientUIDstr)

# Select all series in selected patient
Expand Down Expand Up @@ -864,7 +864,7 @@ def importFromDICOMWeb(dicomWebEndpoint, studyInstanceUID, seriesInstanceUID=Non

seriesList = client.search_for_series(study_instance_uid=studyInstanceUID)
seriesInstanceUIDs = []
if not seriesInstanceUID is None:
if seriesInstanceUID is not None:
seriesInstanceUIDs = [seriesInstanceUID]
else:
for series in seriesList:
Expand Down
2 changes: 1 addition & 1 deletion Modules/Scripted/DICOMPlugins/DICOMGeAbusPlugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ def getMetadata(self, filePath):
fieldInfo = fieldsInfo[fieldName]
if not fieldInfo['required']:
continue
if not fieldName in fieldValues:
if fieldName not in fieldValues:
raise ValueError(f"Mandatory field {fieldName} was not found")

return fieldValues
Expand Down
2 changes: 1 addition & 1 deletion Modules/Scripted/ExtensionWizard/ExtensionWizard.py
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,7 @@ def loadModules(self, path, depth=1):
for module in modulesToLoad:
rawPath = os.path.dirname(module.path)
path = qt.QDir(rawPath)
if not path in searchPaths:
if path not in searchPaths:
searchPaths.append(path)
rawSearchPaths.append(rawPath)
modified = True
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,8 @@ def updateActions(self):

last = self.topLevelItemCount - 2

self._shiftUpAction.enabled = len(rows) and not 0 in rows
self._shiftDownAction.enabled = len(rows) and not last in rows
self._shiftUpAction.enabled = len(rows) and 0 not in rows
self._shiftDownAction.enabled = len(rows) and last not in rows
self._deleteAction.enabled = True if len(rows) else False

# ---------------------------------------------------------------------------
Expand Down
4 changes: 2 additions & 2 deletions Modules/Scripted/SampleData/SampleData.py
Original file line number Diff line number Diff line change
Expand Up @@ -672,7 +672,7 @@ def downloadFromSource(self, source, maximumAttemptsCount=3):
resultFilePaths.append(filePath)

if loadFileType == 'ZipFile':
if loadFile == False:
if loadFile is False:
resultNodes.append(filePath)
break
outputDir = slicer.mrmlScene.GetCacheManager().GetRemoteCacheDirectory() + "/" + os.path.splitext(os.path.basename(filePath))[0]
Expand All @@ -690,7 +690,7 @@ def downloadFromSource(self, source, maximumAttemptsCount=3):
resultNodes.append(filePath)
break
elif nodeName:
if loadFile == False:
if loadFile is False:
resultNodes.append(filePath)
break
loadedNode = self.loadNode(filePath, nodeName, loadFileType, source.loadFileProperties.copy())
Expand Down
16 changes: 10 additions & 6 deletions Modules/Scripted/SegmentStatistics/SegmentStatistics.py
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,7 @@ def registerPlugin(plugin):
if key.count(".") > 0:
logging.warning("Plugin keys should not contain extra '.' as it might mix pluginname.measurementkey in "
"the parameter node")
if not plugin.__class__ in SegmentStatisticsLogic.registeredPlugins:
if plugin.__class__ not in SegmentStatisticsLogic.registeredPlugins:
SegmentStatisticsLogic.registeredPlugins.append(plugin.__class__)
else:
logging.warning("SegmentStatisticsLogic.registerPlugin will not register plugin because \
Expand Down Expand Up @@ -454,7 +454,7 @@ def computeStatistics(self):
segmentID = visibleSegmentIds.GetValue(segmentIndex)
self.updateStatisticsForSegment(segmentID)
finally:
if not transformedSegmentationNode is None:
if transformedSegmentationNode is not None:
# We made a copy and hardened the segmentation transform
self.getParameterNode().SetParameter("Segmentation", segmentationNode.GetID())
slicer.mrmlScene.RemoveNode(transformedSegmentationNode)
Expand Down Expand Up @@ -549,8 +549,10 @@ def getHeaderNames(self, nonEmptyKeysOnly=True):
if dicomBasedName and 'DICOM.UnitsCode' in info and info['DICOM.UnitsCode']:
entry.SetFromString(info['DICOM.UnitsCode'])
units = entry.GetCodeValue()
if len(units) > 0 and units[0] == '[' and units[-1] == ']': units = units[1:-1]
if len(units) > 0: name += ' [' + units + ']'
if len(units) > 0 and units[0] == '[' and units[-1] == ']':
units = units[1:-1]
if len(units) > 0:
name += ' [' + units + ']'
elif 'units' in info and info['units'] and len(info['units']) > 0:
units = info['units']
name += ' [' + units + ']'
Expand Down Expand Up @@ -805,14 +807,16 @@ def test_SegmentStatisticsPlugins(self):
segStatLogic.exportToTable(resultsTableNode)
segStatLogic.showTable(resultsTableNode)
self.assertEqual(segStatLogic.getStatistics()["Test_2", "LabelmapSegmentStatisticsPlugin.voxel_count"], 9807)
with self.assertRaises(KeyError): segStatLogic.getStatistics()["Test_4", "ScalarVolumeSegmentStatisticsPlugin.voxel count"]
with self.assertRaises(KeyError):
segStatLogic.getStatistics()["Test_4", "ScalarVolumeSegmentStatisticsPlugin.voxel count"]
# assert there are no result for this segment
segStatLogic.updateStatisticsForSegment('Test_4')
segStatLogic.exportToTable(resultsTableNode)
segStatLogic.showTable(resultsTableNode)
self.assertEqual(segStatLogic.getStatistics()["Test_2", "LabelmapSegmentStatisticsPlugin.voxel_count"], 9807)
self.assertEqual(segStatLogic.getStatistics()["Test_4", "LabelmapSegmentStatisticsPlugin.voxel_count"], 380)
with self.assertRaises(KeyError): segStatLogic.getStatistics()["Test_5", "ScalarVolumeSegmentStatisticsPlugin.voxel count"]
with self.assertRaises(KeyError):
segStatLogic.getStatistics()["Test_5", "ScalarVolumeSegmentStatisticsPlugin.voxel count"]
# assert there are no result for this segment

# calculate measurements for all segments
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,14 +122,14 @@ def computeStatistics(self, segmentID):
temp = statFilterOptions
statFilterOptions = []
for option in temp:
if not option in self.obbKeys:
if option not in self.obbKeys:
statFilterOptions.append(option)
statFilterOptions.append("oriented_bounding_box")

temp = requestedOptions
requestedOptions = []
for option in temp:
if not option in self.obbKeys:
if option not in self.obbKeys:
requestedOptions.append(option)
requestedOptions.append("oriented_bounding_box")

Expand All @@ -142,14 +142,14 @@ def computeStatistics(self, segmentID):
temp = statFilterOptions
statFilterOptions = []
for option in temp:
if not option in self.principalAxisKeys:
if option not in self.principalAxisKeys:
statFilterOptions.append(option)
statFilterOptions.append("principal_axes")

temp = requestedOptions
requestedOptions = []
for option in temp:
if not option in self.principalAxisKeys:
if option not in self.principalAxisKeys:
requestedOptions.append(option)
requestedOptions.append("principal_axes")
requestedOptions.append("centroid_ras")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,9 +149,12 @@ def createDefaultOptionsWidget(self):
info = self.getMeasurementInfo(key)
if info and ("name" in info or "description" in info):
label = info["name"] if "name" in info else info["description"]
if "name" in info: tooltip += "\nname: " + str(info["name"])
if "description" in info: tooltip += "\ndescription: " + str(info["description"])
if "units" in info: tooltip += "\nunits: " + (str(info["units"]) if info["units"] else "n/a")
if "name" in info:
tooltip += "\nname: " + str(info["name"])
if "description" in info:
tooltip += "\ndescription: " + str(info["description"])
if "units" in info:
tooltip += "\nunits: " + (str(info["units"]) if info["units"] else "n/a")
checkbox = qt.QCheckBox(label, self.optionsWidget)
checkbox.checked = key in requestedKeys
checkbox.setToolTip(tooltip)
Expand Down
Loading

0 comments on commit 34696fb

Please sign in to comment.