[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