Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pr15 split part3 #18

Merged
merged 4 commits into from
Nov 6, 2023
Merged

Pr15 split part3 #18

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a670188
Open subprocess with universal_newlines=True
qinz0822 Oct 25, 2023
c0c47ac
Use '//' as integer division
qinz0822 Nov 3, 2023
eb7eccc
Use key parameter in sort() for sorting purposes
qinz0822 Nov 3, 2023
0cb382b
Misc changes
qinz0822 Nov 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion XSConsoleCurses.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ def AddWrappedText(self, inString, inX, inY, inColour = None):
yPos += 1

def AddHCentredText(self, inString, inY, inColour = None):
xStart = self.xSize / 2 - len(inString) / 2
xStart = self.xSize // 2 - len(inString) // 2
self.ClippedAddStr(inString, xStart, inY, inColour)

def Decorate(self):
Expand Down
2 changes: 1 addition & 1 deletion XSConsoleData.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ def convertPIF(inPIF):
pif['metrics']['device_name'] = Lang('<Unknown>')

# Sort PIFs by device name for consistent order
self.data['host']['PIFs'].sort(lambda x, y : cmp(x['device'], y['device']))
self.data['host']['PIFs'].sort(key=lambda pif: pif['device'])

def convertVBD(inVBD):
retVBD = self.session.xenapi.VBD.get_record(inVBD)
Expand Down
4 changes: 2 additions & 2 deletions XSConsoleDataUtils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ def DeviceList(cls, inWritableOnly):
name = "%-50s%10.10s%10.10s" % (nameDesc[:50], nameLabel[:10], nameSize[:10])
retVal.append(Struct(name = name, vdi = vdi))

retVal.sort(lambda x, y : cmp(x.vdi['name_label'], y.vdi['name_label']))
retVal.sort(key=lambda data: data.vdi['name_label'])

return retVal

Expand Down Expand Up @@ -466,7 +466,7 @@ def SRList(cls, inMode = None, inCapabilities = None):
dataSR['opaqueref'] = sr.HotOpaqueRef().OpaqueRef()
retVal.append( Struct(name = name, sr = dataSR) )

retVal.sort(lambda x, y : cmp(x.name, y.name))
retVal.sort(key=lambda data: data.name)

return retVal

Expand Down
4 changes: 2 additions & 2 deletions XSConsoleDialoguePane.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@ def Update(self, inArranger):
self.xSize = min(inArranger.XBounds(), self.parent.XSize() - self.SHRINKVALUE)
self.xSize = (self.xSize + 1) & ~1 # make xSize even
self.ySize = min(inArranger.YBounds(), self.parent.YSize() - self.SHRINKVALUE)
self.xPos = self.parent.XPos() + (self.parent.XSize() - self.xSize) / 2
self.yPos = self.parent.YPos() + (self.parent.YSize() - self.ySize) / 2
self.xPos = self.parent.XPos() + (self.parent.XSize() - self.xSize) // 2
self.yPos = self.parent.YPos() + (self.parent.YSize() - self.ySize) // 2

class DialoguePane:
def __init__(self, inParent = None, inSizer = None):
Expand Down
2 changes: 1 addition & 1 deletion XSConsoleFields.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ def Render(self, inPane, inXPos, inYPos):
yPos = inYPos
for line in self.wrappedText:
if self.centred:
offset = (self.wrappedWidth - len(line)) / 2
offset = (self.wrappedWidth - len(line)) // 2
inPane.AddText(line, inXPos+offset, yPos, self.colour)
else:
inPane.AddText(line, inXPos, yPos, self.colour)
Expand Down
6 changes: 1 addition & 5 deletions XSConsoleImporter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,12 +113,8 @@ def ActivateNamedPlugIn(cls, inName, *inParams):
@classmethod
def CallReadyHandlers(cls):
# Sort plugins in descending priority order with a default of 1000
def CmpPlugin(x, y):
return cmp(y.get('readyhandlerpriority', 1000),
x.get('readyhandlerpriority', 1000))

plugins = cls.plugIns.values()
plugins.sort(CmpPlugin)
plugins.sort(key=lambda p: p.get('readyhandlerpriority', 1000), reverse=True)

for plugin in plugins:
handler = plugin.get('readyhandler', None)
Expand Down
4 changes: 2 additions & 2 deletions XSConsoleLayout.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,8 @@ def WriteParentOffset(self, inParent):

# Centralise subsequent windows
inParent.OffsetSet(
(inParent.XSize() - self.APP_XSIZE) / 2,
(inParent.YSize() - self.APP_YSIZE) / 2)
(inParent.XSize() - self.APP_XSIZE) // 2,
(inParent.YSize() - self.APP_YSIZE) // 2)

def Create(self):
self.windows.append(CursesWindow(0,1,self.APP_XSIZE, self.APP_YSIZE-1, self.parent)) # MainWindow
Expand Down
2 changes: 1 addition & 1 deletion XSConsoleMenus.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ def AddChoiceDef(self, inChoiceDef, inPriority = None):
inChoiceDef.priority = priority # FIXME (modifies input parameter)
self.choiceDefs.append(inChoiceDef)

self.choiceDefs.sort(lambda x, y : cmp(x.priority, y.priority))
self.choiceDefs.sort(key=lambda c: c.priority)

def AddChoice(self, name, onAction = None, onEnter = None, priority = None, statusUpdateHandler = None, handle = None):
choiceDef = ChoiceDef(name, onAction, onEnter, priority, statusUpdateHandler, handle)
Expand Down
1 change: 1 addition & 0 deletions XSConsoleUtils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ def _NewPipe(self, *inParams):
stdin=subprocess.PIPE,
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
universal_newlines=True,
close_fds=True)
self.called = False

Expand Down
2 changes: 1 addition & 1 deletion plugins-base/XSFeatureEULA.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ def __init__(self, inFilename):
self.maxLine = 0
for line in contents.split('\n'):
self.maxLine = max(self.maxLine, len(line))
self.padding = ' ' * max(0, (xSize - 4 - self.maxLine) / 2)
self.padding = ' ' * max(0, (xSize - 4 - self.maxLine) // 2)

self.text = Lang("End User License Agreement")
self.info = contents
Expand Down
2 changes: 1 addition & 1 deletion plugins-base/XSFeatureSRCommon.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ def __init__(self, inSRHandle):

choiceList = [ name for name in allowedOps if name in SRUtils.AllowedOperations() ]

choiceList.sort(lambda x, y: cmp(SRUtils.OperationPriority(x), SRUtils.OperationPriority(y)))
choiceList.sort(key=lambda choice: SRUtils.OperationPriority(choice))

self.controlMenu = Menu()
for choice in choiceList:
Expand Down
2 changes: 1 addition & 1 deletion plugins-base/XSFeatureSRInfo.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ def MenuRegenerator(cls, inList, inMenu):
srList = [ sr for sr in HotAccessor().visible_sr if sr.other_config({}).get('xensource_internal', '') != 'true' ]

# Sort list by SR shared flag then name
srList.sort(lambda x, y: cmp(y.shared(False), x.shared(False)) or cmp (x.name_label(''), y.name_label()))
srList.sort(key=lambda sr: (-sr.shared(False), sr.name_label('')))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is -sr right here?

Personally, I think the logic would be easier to follow as

# Sort list by SR shared flag then name
srList.sort(key=lambda sr: sr.name_label(''))
srList.sort(key=lambda sr: sr.shared(False))

maybe with a reverse=True in the second line if I've interpreted the intent of the negation properly ?

Copy link
Contributor

@andyhhp andyhhp Nov 3, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively, srList.sort(key=lambda sr: (not sr.shared(False), sr.name_label(''))) might be better ? At least it makes clear that the first part of the tuple is supposed to be a boolean.

As a tangent, passing the default-if-missing value in makes this code horrible to read and follow. Do you happen to know if this is a local artifact, or a property of the API?

Copy link
Collaborator

@bernhardkaindl bernhardkaindl Nov 3, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is -sr right here?

Yes, it is correct. The 1st half of the original cmp(y.shared(False), x.shared(False)) had y and x reversed, so the primary sort order is reversed.

Splitting the nested sort into two consecutive sorts would make the result reliant of the sort algorithm chosen by the Python interpreter for the given data and I'd like the result to be kept clearly defined as a nested sort with two sort keys. Therefore, I see the change is correct!

Because the sort is already commented using a meaningful comment, the sort does not need further explanation:

# Sort list by SR shared flag then name
srList.sort(key=lambda sr: (-sr.shared(False), sr.name_label('')))

Even though the comment says that sr.shared(False) returns a flag, nothing here ensures that it will always return a boolean. In principle, it could return anything (numbers, floats or a mix of all that). Therefore I think keeping -sr.shared(False) is the safe thing to do (does not change the behavior at all) and it's clear that - means negation.


srUtils = Importer.GetResource('SRUtils')
for sr in srList:
Expand Down
4 changes: 2 additions & 2 deletions plugins-base/XSFeatureVMCommon.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ def __init__(self, inVMHandle):

choiceList = [ name for name in allowedOps if name in VMUtils.AllowedOperations() ]

choiceList.sort(lambda x, y: cmp(VMUtils.OperationPriority(x), VMUtils.OperationPriority(y)))
choiceList.sort(key=lambda choice: VMUtils.OperationPriority(choice))

self.controlMenu = Menu()
for choice in choiceList:
Expand All @@ -146,7 +146,7 @@ def BuildPane(self):

if self.state == 'MIGRATE':
hosts = VMUtils.GetPossibleHostAccessors(self.vmHandle)
hosts.sort(lambda x, y: cmp(x.name_label(), y.name_label()))
hosts.sort(key=lambda host: host.name_label())
self.hostMenu = Menu()
residentHost = HotAccessor().vm[self.vmHandle].resident_on()
for host in hosts:
Expand Down
2 changes: 1 addition & 1 deletion plugins-base/XSFeatureVMInfo.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ def MenuRegenerator(cls, inList, inMenu):
# inList is a list of HotOpaqueRef objects
vmList = [ HotAccessor().vm[x] for x in inList ]
# Sort list by VM name
vmList.sort(lambda x,y: cmp(x.name_label(''), y.name_label('')))
vmList.sort(key=lambda vm: vm.name_label(''))

for vm in vmList:
nameLabel = vm.name_label(Lang('<Unknown>'))
Expand Down