Patchwork [7,of,7,clowncopter] automv: switch to specifying the similarity as an integer (0-100)

login
register
mail settings
Submitter Martijn Pieters
Date Feb. 15, 2016, 5:25 p.m.
Message ID <d63407a4948f5a9ef8dd.1455557101@mjpieters-mbp>
Download mbox | patch
Permalink /patch/13209/
State Accepted
Headers show

Comments

Martijn Pieters - Feb. 15, 2016, 5:25 p.m.
# HG changeset patch
# User Martijn Pieters <mjpieters@fb.com>
# Date 1455557082 0
#      Mon Feb 15 17:24:42 2016 +0000
# Node ID d63407a4948f5a9ef8ddcdb077546c889dc4ce07
# Parent  4a57546044d26cc4b2725d404ee60a11ccba3cf9
automv: switch to specifying the similarity as an integer (0-100).

This is consistent with the addremove --similarity option.
Pierre-Yves David - Feb. 15, 2016, 8:27 p.m.
On 02/15/2016 05:25 PM, Martijn Pieters wrote:
> # HG changeset patch
> # User Martijn Pieters <mjpieters@fb.com>
> # Date 1455557082 0
> #      Mon Feb 15 17:24:42 2016 +0000
> # Node ID d63407a4948f5a9ef8ddcdb077546c889dc4ce07
> # Parent  4a57546044d26cc4b2725d404ee60a11ccba3cf9
> automv: switch to specifying the similarity as an integer (0-100).

Pushed to the clowncopter, thanks.
Yuya Nishihara - Feb. 16, 2016, 2:31 p.m.
On Mon, 15 Feb 2016 17:25:01 +0000, Martijn Pieters wrote:
> # HG changeset patch
> # User Martijn Pieters <mjpieters@fb.com>
> # Date 1455557082 0
> #      Mon Feb 15 17:24:42 2016 +0000
> # Node ID d63407a4948f5a9ef8ddcdb077546c889dc4ce07
> # Parent  4a57546044d26cc4b2725d404ee60a11ccba3cf9
> automv: switch to specifying the similarity as an integer (0-100).
> 
> This is consistent with the addremove --similarity option.
> 
> diff --git a/hgext/automv.py b/hgext/automv.py
> --- a/hgext/automv.py
> +++ b/hgext/automv.py
> @@ -10,7 +10,8 @@
>  comes from an unrecorded mv.
>  
>  The threshold at which a file is considered a move can be set with the
> -``automv.similarity`` config option; the default value is 1.00.
> +``automv.similarity`` config option. This option takes a percentage between 0
> +(disabled) and 100 (files must be identical), the default is 100.
>  
>  """
>  from __future__ import absolute_import
> @@ -36,11 +37,12 @@
>      renames = None
>      disabled = opts.pop('no_automv', False)
>      if not disabled:
> -        threshold = float(ui.config('automv', 'similarity', '1.00'))
> +        threshold = float(ui.config('automv', 'similarity', '100'))

ui.configint() can be used to avoid naked exception.
Augie Fackler - Feb. 22, 2016, 10:58 p.m.
On Mon, Feb 15, 2016 at 05:25:01PM +0000, Martijn Pieters wrote:
> # HG changeset patch
> # User Martijn Pieters <mjpieters@fb.com>
> # Date 1455557082 0
> #      Mon Feb 15 17:24:42 2016 +0000
> # Node ID d63407a4948f5a9ef8ddcdb077546c889dc4ce07
> # Parent  4a57546044d26cc4b2725d404ee60a11ccba3cf9
> automv: switch to specifying the similarity as an integer (0-100).

FYI, both for you and other reviewers: this patch missed some fixes to
test-automv.t. I've pushed a fix to the clowncopter (replaced two
instances of 0.6 with 60).

>
> This is consistent with the addremove --similarity option.
>
> diff --git a/hgext/automv.py b/hgext/automv.py
> --- a/hgext/automv.py
> +++ b/hgext/automv.py
> @@ -10,7 +10,8 @@
>  comes from an unrecorded mv.
>
>  The threshold at which a file is considered a move can be set with the
> -``automv.similarity`` config option; the default value is 1.00.
> +``automv.similarity`` config option. This option takes a percentage between 0
> +(disabled) and 100 (files must be identical), the default is 100.
>
>  """
>  from __future__ import absolute_import
> @@ -36,11 +37,12 @@
>      renames = None
>      disabled = opts.pop('no_automv', False)
>      if not disabled:
> -        threshold = float(ui.config('automv', 'similarity', '1.00'))
> +        threshold = float(ui.config('automv', 'similarity', '100'))
>          if threshold > 0:
>              match = scmutil.match(repo[None], pats, opts)
>              added, removed = _interestingfiles(repo, match)
> -            renames = _findrenames(repo, match, added, removed, threshold)
> +            renames = _findrenames(repo, match, added, removed,
> +                                   threshold / 100.0)
>
>      with repo.wlock():
>          if renames is not None:
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Augie Fackler - Feb. 22, 2016, 11:06 p.m.
> On Feb 22, 2016, at 17:58, Augie Fackler <raf@durin42.com> wrote:
> 
> On Mon, Feb 15, 2016 at 05:25:01PM +0000, Martijn Pieters wrote:
>> # HG changeset patch
>> # User Martijn Pieters <mjpieters@fb.com>
>> # Date 1455557082 0
>> #      Mon Feb 15 17:24:42 2016 +0000
>> # Node ID d63407a4948f5a9ef8ddcdb077546c889dc4ce07
>> # Parent  4a57546044d26cc4b2725d404ee60a11ccba3cf9
>> automv: switch to specifying the similarity as an integer (0-100).
> 
> FYI, both for you and other reviewers: this patch missed some fixes to
> test-automv.t. I've pushed a fix to the clowncopter (replaced two
> instances of 0.6 with 60).

Ah, marmoute just pointed out that the problem was exposed by a more recent patch that I just queued, so the tests still passed here.
Pierre-Yves David - Feb. 22, 2016, 11:06 p.m.
On 02/22/2016 11:58 PM, Augie Fackler wrote:
> On Mon, Feb 15, 2016 at 05:25:01PM +0000, Martijn Pieters wrote:
>> # HG changeset patch
>> # User Martijn Pieters <mjpieters@fb.com>
>> # Date 1455557082 0
>> #      Mon Feb 15 17:24:42 2016 +0000
>> # Node ID d63407a4948f5a9ef8ddcdb077546c889dc4ce07
>> # Parent  4a57546044d26cc4b2725d404ee60a11ccba3cf9
>> automv: switch to specifying the similarity as an integer (0-100).
>
> FYI, both for you and other reviewers: this patch missed some fixes to
> test-automv.t. I've pushed a fix to the clowncopter (replaced two
> instances of 0.6 with 60).

Tests run "fine" until the latest patch of martjin (95 similarity as 
default) you queued a couple of minute ago.

There is something fishy here.
Augie Fackler - Feb. 22, 2016, 11:07 p.m.
> On Feb 22, 2016, at 18:06, Pierre-Yves David <pierre-yves.david@ens-lyon.org> wrote:
> 
> 
> 
> On 02/22/2016 11:58 PM, Augie Fackler wrote:
>> On Mon, Feb 15, 2016 at 05:25:01PM +0000, Martijn Pieters wrote:
>>> # HG changeset patch
>>> # User Martijn Pieters <mjpieters@fb.com>
>>> # Date 1455557082 0
>>> #      Mon Feb 15 17:24:42 2016 +0000
>>> # Node ID d63407a4948f5a9ef8ddcdb077546c889dc4ce07
>>> # Parent  4a57546044d26cc4b2725d404ee60a11ccba3cf9
>>> automv: switch to specifying the similarity as an integer (0-100).
>> 
>> FYI, both for you and other reviewers: this patch missed some fixes to
>> test-automv.t. I've pushed a fix to the clowncopter (replaced two
>> instances of 0.6 with 60).
> 
> Tests run "fine" until the latest patch of martjin (95 similarity as default) you queued a couple of minute ago.
> 
> There is something fishy here.

It's because that patch switches (righteously) to using configint. In an ideal world I suppose that'd be two patches, but I'm not going to fuss over it today. I've got bigger fish to fry.

> 
> -- 
> Pierre-Yves David
Martijn Pieters - Feb. 23, 2016, 11:48 a.m.
On 22 February 2016 at 22:58, Augie Fackler <raf@durin42.com> wrote:
> FYI, both for you and other reviewers: this patch missed some fixes to
> test-automv.t. I've pushed a fix to the clowncopter (replaced two
> instances of 0.6 with 60).

Mea culpa, I had a separate patch that corrected that that should have
gone out with the one you pushed. Thanks for correcting this!

Patch

diff --git a/hgext/automv.py b/hgext/automv.py
--- a/hgext/automv.py
+++ b/hgext/automv.py
@@ -10,7 +10,8 @@ 
 comes from an unrecorded mv.
 
 The threshold at which a file is considered a move can be set with the
-``automv.similarity`` config option; the default value is 1.00.
+``automv.similarity`` config option. This option takes a percentage between 0
+(disabled) and 100 (files must be identical), the default is 100.
 
 """
 from __future__ import absolute_import
@@ -36,11 +37,12 @@ 
     renames = None
     disabled = opts.pop('no_automv', False)
     if not disabled:
-        threshold = float(ui.config('automv', 'similarity', '1.00'))
+        threshold = float(ui.config('automv', 'similarity', '100'))
         if threshold > 0:
             match = scmutil.match(repo[None], pats, opts)
             added, removed = _interestingfiles(repo, match)
-            renames = _findrenames(repo, match, added, removed, threshold)
+            renames = _findrenames(repo, match, added, removed,
+                                   threshold / 100.0)
 
     with repo.wlock():
         if renames is not None: