Patchwork [3,of,6] py3: implement __lt__() to make gitlfspointer sortable

login
register
mail settings
Submitter Matt Harbison
Date Sept. 30, 2018, 5:46 a.m.
Message ID <96752a0137b4b08677f3.1538286382@Envy>
Download mbox | patch
Permalink /patch/35212/
State Accepted
Headers show

Comments

Matt Harbison - Sept. 30, 2018, 5:46 a.m.
# HG changeset patch
# User Matt Harbison <matt_harbison@yahoo.com>
# Date 1538278134 14400
#      Sat Sep 29 23:28:54 2018 -0400
# Node ID 96752a0137b4b08677f3205362ece3c2e7fde057
# Parent  1dd7c3dcfb46a39bc61419415c0818e0ed1bbd1c
py3: implement __lt__() to make gitlfspointer sortable
Yuya Nishihara - Sept. 30, 2018, 12:08 p.m.
On Sun, 30 Sep 2018 01:46:22 -0400, Matt Harbison wrote:
> # HG changeset patch
> # User Matt Harbison <matt_harbison@yahoo.com>
> # Date 1538278134 14400
> #      Sat Sep 29 23:28:54 2018 -0400
> # Node ID 96752a0137b4b08677f3205362ece3c2e7fde057
> # Parent  1dd7c3dcfb46a39bc61419415c0818e0ed1bbd1c
> py3: implement __lt__() to make gitlfspointer sortable
> 
> diff --git a/hgext/lfs/pointer.py b/hgext/lfs/pointer.py
> --- a/hgext/lfs/pointer.py
> +++ b/hgext/lfs/pointer.py
> @@ -30,6 +30,9 @@ class gitlfspointer(dict):
>          super(gitlfspointer, self).__init__(*args)
>          self.update(pycompat.byteskwargs(kwargs))
>  
> +    def __lt__(self, other):
> +        return self.oid() < other.oid()

I don't think it's correct to implement only __lt__(). Perhaps, easier
workaround is to use sorted(..., key=lambda...) instead.

Queued the other patches, thanks.
Matt Harbison - Sept. 30, 2018, 1:20 p.m.
> On Sep 30, 2018, at 8:08 AM, Yuya Nishihara <yuya@tcha.org> wrote:
> 
>> On Sun, 30 Sep 2018 01:46:22 -0400, Matt Harbison wrote:
>> # HG changeset patch
>> # User Matt Harbison <matt_harbison@yahoo.com>
>> # Date 1538278134 14400
>> #      Sat Sep 29 23:28:54 2018 -0400
>> # Node ID 96752a0137b4b08677f3205362ece3c2e7fde057
>> # Parent  1dd7c3dcfb46a39bc61419415c0818e0ed1bbd1c
>> py3: implement __lt__() to make gitlfspointer sortable
>> 
>> diff --git a/hgext/lfs/pointer.py b/hgext/lfs/pointer.py
>> --- a/hgext/lfs/pointer.py
>> +++ b/hgext/lfs/pointer.py
>> @@ -30,6 +30,9 @@ class gitlfspointer(dict):
>>         super(gitlfspointer, self).__init__(*args)
>>         self.update(pycompat.byteskwargs(kwargs))
>> 
>> +    def __lt__(self, other):
>> +        return self.oid() < other.oid()
> 
> I don't think it's correct to implement only __lt__(). Perhaps, easier
> workaround is to use sorted(..., key=lambda...) instead.

The SO post I saw implemented __eq__() too, but then that usually means needing a hash function too.  I didn’t go the route of specifying the sorted() key, because it seems too easy to forget to do that in each place.  (And harder to find each place now.)

> Queued the other patches, thanks.
Yuya Nishihara - Sept. 30, 2018, 1:42 p.m.
On Sun, 30 Sep 2018 09:20:15 -0400, Matt Harbison wrote:
> > On Sep 30, 2018, at 8:08 AM, Yuya Nishihara <yuya@tcha.org> wrote:
> > 
> >> On Sun, 30 Sep 2018 01:46:22 -0400, Matt Harbison wrote:
> >> # HG changeset patch
> >> # User Matt Harbison <matt_harbison@yahoo.com>
> >> # Date 1538278134 14400
> >> #      Sat Sep 29 23:28:54 2018 -0400
> >> # Node ID 96752a0137b4b08677f3205362ece3c2e7fde057
> >> # Parent  1dd7c3dcfb46a39bc61419415c0818e0ed1bbd1c
> >> py3: implement __lt__() to make gitlfspointer sortable
> >> 
> >> diff --git a/hgext/lfs/pointer.py b/hgext/lfs/pointer.py
> >> --- a/hgext/lfs/pointer.py
> >> +++ b/hgext/lfs/pointer.py
> >> @@ -30,6 +30,9 @@ class gitlfspointer(dict):
> >>         super(gitlfspointer, self).__init__(*args)
> >>         self.update(pycompat.byteskwargs(kwargs))
> >> 
> >> +    def __lt__(self, other):
> >> +        return self.oid() < other.oid()
> > 
> > I don't think it's correct to implement only __lt__(). Perhaps, easier
> > workaround is to use sorted(..., key=lambda...) instead.
> 
> The SO post I saw implemented __eq__() too, but then that usually means needing a hash function too.

Yes.

> I didn’t go the route of specifying the sorted() key, because it seems too easy to forget to do that in each place.  (And harder to find each place now.)

Maybe we can run the test with __le__() that raises Exception? It's probably
wrong to sort pointers in memory address order.

To support partial ordering, we'll probably need to implement le, lt, ge, gt.

Patch

diff --git a/hgext/lfs/pointer.py b/hgext/lfs/pointer.py
--- a/hgext/lfs/pointer.py
+++ b/hgext/lfs/pointer.py
@@ -30,6 +30,9 @@  class gitlfspointer(dict):
         super(gitlfspointer, self).__init__(*args)
         self.update(pycompat.byteskwargs(kwargs))
 
+    def __lt__(self, other):
+        return self.oid() < other.oid()
+
     @classmethod
     def deserialize(cls, text):
         try: