[PATCH 2/2] Don't blindly expect that everything is hashable
Vratislav Podzimek
vpodzime at redhat.com
Wed Feb 25 12:27:53 UTC 2015
On Fri, 2015-02-20 at 09:50 -0500, Anne Mulhern wrote:
> ----- Original Message -----
> > From: "Vratislav Podzimek" <vpodzime at redhat.com>
> > To: anaconda-patches at 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 at 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
More information about the anaconda-patches
mailing list