On Fri, 2015-02-20 at 09:50 -0500, Anne Mulhern wrote:
----- Original Message -----
> From: "Vratislav Podzimek" <vpodzime(a)redhat.com>
> To: anaconda-patches(a)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(a)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)
> +
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.
--
Vratislav Podzimek
Anaconda Rider | Red Hat, Inc. | Brno - Czech Republic