With these changes and python3-pyparted installed, blivet tests using pyparted work with Python 3. Whoohooo!
Vratislav Podzimek (2): Replace filter() call with list comprehension Don't blindly expect that everything is hashable
src/parted/alignment.py | 9 ++++++--- src/parted/cachedlist.py | 3 +++ src/parted/device.py | 9 ++++++--- src/parted/disk.py | 11 +++++++---- src/parted/filesystem.py | 9 ++++++--- src/parted/geometry.py | 9 ++++++--- src/parted/partition.py | 9 ++++++--- 7 files changed, 40 insertions(+), 19 deletions(-)
The list comprehension works the same way in both Python 2 and Python 3.
Signed-off-by: Vratislav Podzimek vpodzime@redhat.com --- src/parted/disk.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/parted/disk.py b/src/parted/disk.py index 3f293e2..a450c08 100644 --- a/src/parted/disk.py +++ b/src/parted/disk.py @@ -395,7 +395,7 @@ class Disk(object): return None
def __filterPartitions(self, fn): - return filter(fn, self.partitions) + return [part for part in self.partitions if fn(part)]
def getLogicalPartitions(self): """Return a list of logical Partitions on this Disk."""
----- Original Message -----
From: "Vratislav Podzimek" vpodzime@redhat.com To: anaconda-patches@lists.fedorahosted.org Sent: Friday, February 20, 2015 9:06:56 AM Subject: [PATCH 1/2] Replace filter() call with list comprehension
The list comprehension works the same way in both Python 2 and Python 3.
Signed-off-by: Vratislav Podzimek vpodzime@redhat.com
src/parted/disk.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/parted/disk.py b/src/parted/disk.py index 3f293e2..a450c08 100644 --- a/src/parted/disk.py +++ b/src/parted/disk.py @@ -395,7 +395,7 @@ class Disk(object): return None
def __filterPartitions(self, fn):
return filter(fn, self.partitions)
return [part for part in self.partitions if fn(part)]def getLogicalPartitions(self): """Return a list of logical Partitions on this Disk."""
-- 2.1.0
anaconda-patches mailing list anaconda-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
Ack.
- mulhern
Provide the __hash__ method's for our classes and check that the other objects are hashable before we try to hash them.
Signed-off-by: Vratislav Podzimek vpodzime@redhat.com --- src/parted/alignment.py | 9 ++++++--- src/parted/cachedlist.py | 3 +++ src/parted/device.py | 9 ++++++--- src/parted/disk.py | 9 ++++++--- src/parted/filesystem.py | 9 ++++++--- src/parted/geometry.py | 9 ++++++--- src/parted/partition.py | 9 ++++++--- 7 files changed, 39 insertions(+), 18 deletions(-)
diff --git a/src/parted/alignment.py b/src/parted/alignment.py index ad059dc..452125e 100644 --- a/src/parted/alignment.py +++ b/src/parted/alignment.py @@ -55,12 +55,12 @@ class Alignment(object): return not self.__ne__(other)
def __ne__(self, other): - if hash(self) == hash(other): - return False - if type(self) != type(other): return True
+ if hasattr(other, "__hash__"): + return hash(self) != hash(other) + return self.offset != other.offset or self.grainSize != other.grainSize
def __str__(self): @@ -71,6 +71,9 @@ class Alignment(object): "ped": self.__alignment}) return s
+ def __hash__(self): + return hash(str(self)) + @localeC def intersect(self, b): """Create and return a new Alignment that describes the intersection of diff --git a/src/parted/cachedlist.py b/src/parted/cachedlist.py index 6c522dc..cbf7e39 100644 --- a/src/parted/cachedlist.py +++ b/src/parted/cachedlist.py @@ -75,6 +75,9 @@ class CachedList(Sequence): self.__rebuildList() return str(self._lst)
+ def __hash__(self): + return hash(str(self)) + def count(self, value): self.__rebuildList() return self._lst.count(value) diff --git a/src/parted/device.py b/src/parted/device.py index 8cb24fe..5ae2fe7 100644 --- a/src/parted/device.py +++ b/src/parted/device.py @@ -59,12 +59,12 @@ class Device(object): return not self.__ne__(other)
def __ne__(self, other): - if hash(self) == hash(other): - return False - if type(self) != type(other): return True
+ if hasattr(other, "__hash__"): + return hash(self) != hash(other) + return self.model != other.model or self.path != other.path or self.type != other.type or self.length != other.length
def __getCHS(self, geometry): @@ -182,6 +182,9 @@ class Device(object): "ped": self.__device}) return s
+ def __hash__(self): + return hash(str(self)) + @localeC def clobber(self): """Remove all identifying signatures of the partition table.""" diff --git a/src/parted/disk.py b/src/parted/disk.py index a450c08..3006266 100644 --- a/src/parted/disk.py +++ b/src/parted/disk.py @@ -74,12 +74,12 @@ class Disk(object): return not self.__ne__(other)
def __ne__(self, other): - if hash(self) == hash(other): - return False - if type(self) != type(other): return True
+ if hasattr(other, "__hash__"): + return hash(self) != hash(other) + return self.device != other.device or not self._hasSameParts(other)
def __str__(self): @@ -95,6 +95,9 @@ class Disk(object): "ped": self.__disk}) return s
+ def __hash__(self): + return hash(str(self)) + def __getPartitions(self): """Construct a list of partitions on the disk. This is called only as needed from the self.partitions property, which just happens to be diff --git a/src/parted/filesystem.py b/src/parted/filesystem.py index 3c8045c..9478c45 100644 --- a/src/parted/filesystem.py +++ b/src/parted/filesystem.py @@ -62,12 +62,12 @@ class FileSystem(object): return not self.__ne__(other)
def __ne__(self, other): - if hash(self) == hash(other): - return False - if type(self) != type(other): return True
+ if hasattr(other, "__hash__"): + return hash(self) != hash(other) + return self.type != other.type or self.geometry != other.geometry
def __str__(self): @@ -78,6 +78,9 @@ class FileSystem(object): "checked": self.checked, "ped": self.__fileSystem}) return s
+ def __hash__(self): + return hash(str(self)) + @property def type(self): """The type of this filesystem, e.g. ext3.""" diff --git a/src/parted/geometry.py b/src/parted/geometry.py index 3316ab9..c391abb 100644 --- a/src/parted/geometry.py +++ b/src/parted/geometry.py @@ -67,12 +67,12 @@ class Geometry(object): return not self.__ne__(other)
def __ne__(self, other): - if hash(self) == hash(other): - return False - if type(self) != type(other): return True
+ if hasattr(other, "__hash__"): + return hash(self) != hash(other) + return self.device != other.device or self.start != other.start or self.length != other.length
def __str__(self): @@ -83,6 +83,9 @@ class Geometry(object): "device": self.device, "ped": self.__geometry}) return s
+ def __hash__(self): + return hash(str(self)) + @property def device(self): """The Device this geometry describes.""" diff --git a/src/parted/partition.py b/src/parted/partition.py index 89cb673..a24f9e4 100644 --- a/src/parted/partition.py +++ b/src/parted/partition.py @@ -72,12 +72,12 @@ class Partition(object): return not self.__ne__(other)
def __ne__(self, other): - if hash(self) == hash(other): - return False - if type(self) != type(other): return True
+ if hasattr(other, "__hash__"): + return hash(self) != hash(other) + return self.path != other.path or self.type != other.type or self.geometry != other.geometry or self.fileSystem != other.fileSystem
def __str__(self): @@ -97,6 +97,9 @@ class Partition(object): "busy": self.busy, "ped": self.__partition}) return s
+ def __hash__(self): + return hash(str(self)) + def __writeOnly(self, prop): raise parted.WriteOnlyProperty(prop)
----- Original Message -----
From: "Vratislav Podzimek" vpodzime@redhat.com To: anaconda-patches@lists.fedorahosted.org Sent: Friday, February 20, 2015 9:06:57 AM Subject: [PATCH 2/2] Don't blindly expect that everything is hashable
Provide the __hash__ method's for our classes and check that the other objects are hashable before we try to hash them.
Signed-off-by: Vratislav Podzimek vpodzime@redhat.com
src/parted/alignment.py | 9 ++++++--- src/parted/cachedlist.py | 3 +++ src/parted/device.py | 9 ++++++--- src/parted/disk.py | 9 ++++++--- src/parted/filesystem.py | 9 ++++++--- src/parted/geometry.py | 9 ++++++--- src/parted/partition.py | 9 ++++++--- 7 files changed, 39 insertions(+), 18 deletions(-)
diff --git a/src/parted/alignment.py b/src/parted/alignment.py index ad059dc..452125e 100644 --- a/src/parted/alignment.py +++ b/src/parted/alignment.py @@ -55,12 +55,12 @@ class Alignment(object): return not self.__ne__(other)
def __ne__(self, other):
if hash(self) == hash(other):return Falseif type(self) != type(other): return True
if hasattr(other, "__hash__"):return hash(self) != hash(other)
Might make more sense just to get rid of hash() calls in ne entirely. Previously, all they did was serve as some kind of optimization on object identity, it looks like.
And Alignment does define hash(), since it extends object.
Also, it's easier to reason about correctness of hash() wrt. eq if they don't depend on each other.
Why not just make hash return some synthesis of the two fields, offset and grainSize, that actually mattered to equality in the original?
Or, if you really think that equality should now depend on alignment as well, (I don't know if that makes sense or not), make that explicit in the __ne__ method, rather than hiding it in that change on the hash method.
Generally speaking, it's not really a good idea to make __hash__ be a function of __str__(), AFAIK.
- mulhern
return self.offset != other.offset or self.grainSize != other.grainSize def __str__(self):@@ -71,6 +71,9 @@ class Alignment(object): "ped": self.__alignment}) return s
- def __hash__(self):
return hash(str(self))- @localeC def intersect(self, b): """Create and return a new Alignment that describes the intersection of
diff --git a/src/parted/cachedlist.py b/src/parted/cachedlist.py index 6c522dc..cbf7e39 100644 --- a/src/parted/cachedlist.py +++ b/src/parted/cachedlist.py @@ -75,6 +75,9 @@ class CachedList(Sequence): self.__rebuildList() return str(self._lst)
- def __hash__(self):
return hash(str(self))- def count(self, value): self.__rebuildList() return self._lst.count(value)
diff --git a/src/parted/device.py b/src/parted/device.py index 8cb24fe..5ae2fe7 100644 --- a/src/parted/device.py +++ b/src/parted/device.py @@ -59,12 +59,12 @@ class Device(object): return not self.__ne__(other)
def __ne__(self, other):
if hash(self) == hash(other):return Falseif type(self) != type(other): return True
if hasattr(other, "__hash__"):return hash(self) != hash(other)return self.model != other.model or self.path != other.path or self.type != other.type or self.length != other.lengthdef __getCHS(self, geometry):
@@ -182,6 +182,9 @@ class Device(object): "ped": self.__device}) return s
- def __hash__(self):
return hash(str(self))- @localeC def clobber(self): """Remove all identifying signatures of the partition table."""
diff --git a/src/parted/disk.py b/src/parted/disk.py index a450c08..3006266 100644 --- a/src/parted/disk.py +++ b/src/parted/disk.py @@ -74,12 +74,12 @@ class Disk(object): return not self.__ne__(other)
def __ne__(self, other):
if hash(self) == hash(other):return Falseif type(self) != type(other): return True
if hasattr(other, "__hash__"):return hash(self) != hash(other)return self.device != other.device or not self._hasSameParts(other)def __str__(self):
@@ -95,6 +95,9 @@ class Disk(object): "ped": self.__disk}) return s
- def __hash__(self):
return hash(str(self))- def __getPartitions(self): """Construct a list of partitions on the disk. This is called only as needed from the self.partitions property, which just happens to be
diff --git a/src/parted/filesystem.py b/src/parted/filesystem.py index 3c8045c..9478c45 100644 --- a/src/parted/filesystem.py +++ b/src/parted/filesystem.py @@ -62,12 +62,12 @@ class FileSystem(object): return not self.__ne__(other)
def __ne__(self, other):
if hash(self) == hash(other):return Falseif type(self) != type(other): return True
if hasattr(other, "__hash__"):return hash(self) != hash(other)return self.type != other.type or self.geometry != other.geometrydef __str__(self):
@@ -78,6 +78,9 @@ class FileSystem(object): "checked": self.checked, "ped": self.__fileSystem}) return s
- def __hash__(self):
return hash(str(self))- @property def type(self): """The type of this filesystem, e.g. ext3."""
diff --git a/src/parted/geometry.py b/src/parted/geometry.py index 3316ab9..c391abb 100644 --- a/src/parted/geometry.py +++ b/src/parted/geometry.py @@ -67,12 +67,12 @@ class Geometry(object): return not self.__ne__(other)
def __ne__(self, other):
if hash(self) == hash(other):return Falseif type(self) != type(other): return True
if hasattr(other, "__hash__"):return hash(self) != hash(other)return self.device != other.device or self.start != other.start or self.length != other.lengthdef __str__(self):
@@ -83,6 +83,9 @@ class Geometry(object): "device": self.device, "ped": self.__geometry}) return s
- def __hash__(self):
return hash(str(self))- @property def device(self): """The Device this geometry describes."""
diff --git a/src/parted/partition.py b/src/parted/partition.py index 89cb673..a24f9e4 100644 --- a/src/parted/partition.py +++ b/src/parted/partition.py @@ -72,12 +72,12 @@ class Partition(object): return not self.__ne__(other)
def __ne__(self, other):
if hash(self) == hash(other):return Falseif type(self) != type(other): return True
if hasattr(other, "__hash__"):return hash(self) != hash(other)return self.path != other.path or self.type != other.type or self.geometry != other.geometry or self.fileSystem != other.fileSystemdef __str__(self):
@@ -97,6 +97,9 @@ class Partition(object): "busy": self.busy, "ped": self.__partition}) return s
- def __hash__(self):
return hash(str(self))- def __writeOnly(self, prop): raise parted.WriteOnlyProperty(prop)
-- 2.1.0
anaconda-patches mailing list anaconda-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
On Fri, 2015-02-20 at 09:50 -0500, Anne Mulhern wrote:
----- Original Message -----
From: "Vratislav Podzimek" vpodzime@redhat.com To: anaconda-patches@lists.fedorahosted.org Sent: Friday, February 20, 2015 9:06:57 AM Subject: [PATCH 2/2] Don't blindly expect that everything is hashable
Provide the __hash__ method's for our classes and check that the other objects are hashable before we try to hash them.
Signed-off-by: Vratislav Podzimek vpodzime@redhat.com
src/parted/alignment.py | 9 ++++++--- src/parted/cachedlist.py | 3 +++ src/parted/device.py | 9 ++++++--- src/parted/disk.py | 9 ++++++--- src/parted/filesystem.py | 9 ++++++--- src/parted/geometry.py | 9 ++++++--- src/parted/partition.py | 9 ++++++--- 7 files changed, 39 insertions(+), 18 deletions(-)
diff --git a/src/parted/alignment.py b/src/parted/alignment.py index ad059dc..452125e 100644 --- a/src/parted/alignment.py +++ b/src/parted/alignment.py @@ -55,12 +55,12 @@ class Alignment(object): return not self.__ne__(other)
def __ne__(self, other):
if hash(self) == hash(other):return Falseif type(self) != type(other): return True
if hasattr(other, "__hash__"):return hash(self) != hash(other)Might make more sense just to get rid of hash() calls in ne entirely. Previously, all they did was serve as some kind of optimization on object identity, it looks like.
That would be best, but I don't understand the pyparted code enough to decide what __eq__ and __ne__ should consider.
And Alignment does define hash(), since it extends object.
Maybe we could just make all pyparted classes proper "new-style" classes and make them all extend object.
Also, it's easier to reason about correctness of hash() wrt. eq if they don't depend on each other.
Why not just make hash return some synthesis of the two fields, offset and grainSize, that actually mattered to equality in the original?
Again, not an expert here.
Or, if you really think that equality should now depend on alignment as well, (I don't know if that makes sense or not), make that explicit in the __ne__ method, rather than hiding it in that change on the hash method.
Generally speaking, it's not really a good idea to make __hash__ be a function of __str__(), AFAIK.
I think it should keep the old "object identity" behaviour and make things Python3-compatible which was my goal here.
anaconda-patches@lists.fedorahosted.org
-
Anne Mulhern -
Vratislav Podzimek