D7384: commands: necessary annotations and suppresssions to pass pytype

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
12 messages Options
Reply | Threaded
Open this post in threaded view
|

D7384: commands: necessary annotations and suppresssions to pass pytype

mharbison72 (Matt Harbison)
durin42 created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  As with other places, there are some places where our types are just
  too complicated for pytype, so we put some suppressions in place.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D7384

AFFECTED FILES
  mercurial/commands.py

CHANGE DETAILS

diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -72,6 +72,15 @@
     stringutil,
 )
 
+if not globals():
+    from typing import (
+        Any,
+        Dict,
+    )
+
+    for t in (Any, Dict):
+        assert t
+
 table = {}
 table.update(debugcommandsmod.command._table)
 
@@ -1108,7 +1117,14 @@
         finally:
             state[b'current'] = [node]
             hbisect.save_state(repo, state)
-        hbisect.printresult(ui, repo, state, displayer, nodes, bgood)
+        hbisect.printresult(
+            ui,
+            repo,
+            state,
+            displayer,
+            nodes,
+            bgood,  # pytype: disable=name-error
+        )
         return
 
     hbisect.checkstate(state)
@@ -2970,7 +2986,7 @@
     if opts.get(b'base'):
         basectx = scmutil.revsingle(repo, opts[b'base'], None)
     # a dict of data to be stored in state file
-    statedata = {}
+    statedata = {}  # type: Dict[bytes, Any]
     # list of new nodes created by ongoing graft
     statedata[b'newnodes'] = []
 
@@ -3240,7 +3256,8 @@
                 )
             # checking that newnodes exist because old state files won't have it
             elif statedata.get(b'newnodes') is not None:
-                statedata[b'newnodes'].append(node)
+                l = statedata[b'newnodes']
+                l.append(node)  # pytype: disable=attribute-error
 
     # remove state when we complete successfully
     if not opts.get(b'dry_run'):
@@ -4731,7 +4748,11 @@
     if copies:
         endrev = None
         if revs:
-            endrev = revs.max() + 1
+            # If we're here, we know revs is a smartset.baseset
+            # because we're not possibly in the linerange mode. Sadly,
+            # pytype isn't convinced, so we have to suppress the
+            # warning about list not having a max() method.
+            endrev = revs.max() + 1  # pytype: disable=attribute-error
         getcopies = scmutil.getcopiesfn(repo, endrev=endrev)
 
     ui.pager(b'log')
@@ -7177,11 +7198,13 @@
         t = []
         if incoming:
             t.append(_(b'1 or more incoming'))
-        o = outgoing.missing
+        o = outgoing.missing  # pytype: disable=attribute-error
         if o:
             t.append(_(b'%d outgoing') % len(o))
         other = dother or sother
-        if b'bookmarks' in other.listkeys(b'namespaces'):
+        if b'bookmarks' in other.listkeys(  # pytype: disable=attribute-error
+            b'namespaces'
+        ):
             counts = bookmarks.summary(repo, other)
             if counts[0] > 0:
                 t.append(_(b'%d incoming bookmarks') % counts[0])



To: durin42, #hg-reviewers
Cc: mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D7384: commands: necessary annotations and suppresssions to pass pytype

mharbison72 (Matt Harbison)
dlax added inline comments.

INLINE COMMENTS

> commands.py:1127
> +            bgood,  # pytype: disable=name-error
> +        )
>          return

This one is sad. I think this can be sorted out by replacing the `try:/finally:` with a context manager. (Can send a patch, if it sounds good to you.)

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7384/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7384

To: durin42, #hg-reviewers
Cc: dlax, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D7384: commands: necessary annotations and suppresssions to pass pytype

mharbison72 (Matt Harbison)
In reply to this post by mharbison72 (Matt Harbison)
durin42 updated this revision to Diff 18099.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D7384?vs=18071&id=18099

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7384/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7384

AFFECTED FILES
  mercurial/commands.py

CHANGE DETAILS

diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -72,6 +72,15 @@
     stringutil,
 )
 
+if not globals():
+    from typing import (
+        Any,
+        Dict,
+    )
+
+    for t in (Any, Dict):
+        assert t
+
 table = {}
 table.update(debugcommandsmod.command._table)
 
@@ -1108,7 +1117,14 @@
         finally:
             state[b'current'] = [node]
             hbisect.save_state(repo, state)
-        hbisect.printresult(ui, repo, state, displayer, nodes, bgood)
+        hbisect.printresult(
+            ui,
+            repo,
+            state,
+            displayer,
+            nodes,
+            bgood,  # pytype: disable=name-error
+        )
         return
 
     hbisect.checkstate(state)
@@ -2970,7 +2986,7 @@
     if opts.get(b'base'):
         basectx = scmutil.revsingle(repo, opts[b'base'], None)
     # a dict of data to be stored in state file
-    statedata = {}
+    statedata = {}  # type: Dict[bytes, Any]
     # list of new nodes created by ongoing graft
     statedata[b'newnodes'] = []
 
@@ -3240,7 +3256,8 @@
                 )
             # checking that newnodes exist because old state files won't have it
             elif statedata.get(b'newnodes') is not None:
-                statedata[b'newnodes'].append(node)
+                l = statedata[b'newnodes']
+                l.append(node)  # pytype: disable=attribute-error
 
     # remove state when we complete successfully
     if not opts.get(b'dry_run'):
@@ -4728,7 +4745,11 @@
     if opts.get(b'copies'):
         endrev = None
         if revs:
-            endrev = revs.max() + 1
+            # If we're here, we know revs is a smartset.baseset
+            # because we're not possibly in the linerange mode. Sadly,
+            # pytype isn't convinced, so we have to suppress the
+            # warning about list not having a max() method.
+            endrev = revs.max() + 1  # pytype: disable=attribute-error
         getcopies = scmutil.getcopiesfn(repo, endrev=endrev)
 
     ui.pager(b'log')
@@ -7178,11 +7199,13 @@
         t = []
         if incoming:
             t.append(_(b'1 or more incoming'))
-        o = outgoing.missing
+        o = outgoing.missing  # pytype: disable=attribute-error
         if o:
             t.append(_(b'%d outgoing') % len(o))
         other = dother or sother
-        if b'bookmarks' in other.listkeys(b'namespaces'):
+        if b'bookmarks' in other.listkeys(  # pytype: disable=attribute-error
+            b'namespaces'
+        ):
             counts = bookmarks.summary(repo, other)
             if counts[0] > 0:
                 t.append(_(b'%d incoming bookmarks') % counts[0])



To: durin42, #hg-reviewers
Cc: dlax, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D7384: commands: necessary annotations and suppresssions to pass pytype

mharbison72 (Matt Harbison)
In reply to this post by mharbison72 (Matt Harbison)
durin42 added inline comments.

INLINE COMMENTS

> dlax wrote in commands.py:1127
> This one is sad. I think this can be sorted out by replacing the `try:/finally:` with a context manager. (Can send a patch, if it sounds good to you.)

By all means!

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7384/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7384

To: durin42, #hg-reviewers
Cc: dlax, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D7384: commands: necessary annotations and suppresssions to pass pytype

mharbison72 (Matt Harbison)
In reply to this post by mharbison72 (Matt Harbison)
This revision now requires changes to proceed.
dlax added a comment.
dlax requested changes to this revision.


  D7430 <https://phab.mercurial-scm.org/D7430> makes this changes unnecessary I think.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7384/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7384

To: durin42, #hg-reviewers, dlax
Cc: dlax, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D7384: commands: necessary annotations and suppresssions to pass pytype

mharbison72 (Matt Harbison)
In reply to this post by mharbison72 (Matt Harbison)
durin42 added a comment.


  I still want to keep the annotations I added. :)

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7384/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7384

To: durin42, #hg-reviewers, dlax
Cc: dlax, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D7384: commands: necessary annotations and suppresssions to pass pytype

mharbison72 (Matt Harbison)
In reply to this post by mharbison72 (Matt Harbison)
durin42 updated this revision to Diff 18162.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D7384?vs=18099&id=18162

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7384/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7384

AFFECTED FILES
  mercurial/commands.py

CHANGE DETAILS

diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -72,6 +72,15 @@
     stringutil,
 )
 
+if not globals():
+    from typing import (
+        Any,
+        Dict,
+    )
+
+    for t in (Any, Dict):
+        assert t
+
 table = {}
 table.update(debugcommandsmod.command._table)
 
@@ -2967,7 +2976,7 @@
     if opts.get(b'base'):
         basectx = scmutil.revsingle(repo, opts[b'base'], None)
     # a dict of data to be stored in state file
-    statedata = {}
+    statedata = {}  # type: Dict[bytes, Any]
     # list of new nodes created by ongoing graft
     statedata[b'newnodes'] = []
 
@@ -3237,7 +3246,8 @@
                 )
             # checking that newnodes exist because old state files won't have it
             elif statedata.get(b'newnodes') is not None:
-                statedata[b'newnodes'].append(node)
+                l = statedata[b'newnodes']
+                l.append(node)  # pytype: disable=attribute-error
 
     # remove state when we complete successfully
     if not opts.get(b'dry_run'):
@@ -4725,7 +4735,11 @@
     if opts.get(b'copies'):
         endrev = None
         if revs:
-            endrev = revs.max() + 1
+            # If we're here, we know revs is a smartset.baseset
+            # because we're not possibly in the linerange mode. Sadly,
+            # pytype isn't convinced, so we have to suppress the
+            # warning about list not having a max() method.
+            endrev = revs.max() + 1  # pytype: disable=attribute-error
         getcopies = scmutil.getcopiesfn(repo, endrev=endrev)
 
     ui.pager(b'log')
@@ -7175,11 +7189,13 @@
         t = []
         if incoming:
             t.append(_(b'1 or more incoming'))
-        o = outgoing.missing
+        o = outgoing.missing  # pytype: disable=attribute-error
         if o:
             t.append(_(b'%d outgoing') % len(o))
         other = dother or sother
-        if b'bookmarks' in other.listkeys(b'namespaces'):
+        if b'bookmarks' in other.listkeys(  # pytype: disable=attribute-error
+            b'namespaces'
+        ):
             counts = bookmarks.summary(repo, other)
             if counts[0] > 0:
                 t.append(_(b'%d incoming bookmarks') % counts[0])



To: durin42, #hg-reviewers, dlax
Cc: dlax, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D7384: commands: necessary annotations and suppresssions to pass pytype

mharbison72 (Matt Harbison)
In reply to this post by mharbison72 (Matt Harbison)
dlax added inline comments.

INLINE COMMENTS

> commands.py:4742
> +            # warning about list not having a max() method.
> +            endrev = revs.max() + 1  # pytype: disable=attribute-error
>          getcopies = scmutil.getcopiesfn(repo, endrev=endrev)

`revs` is always a `smartset.baseset` per af9c73f26371 <https://phab.mercurial-scm.org/rHGaf9c73f263713a7d379c629006be6701dd9956c2> so there should be no attribute error. Or is it because `logcmdutil.getlinerangerevs()` has no type annotation (whereas `logcmdutil.getrevs()` has some)?

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7384/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7384

To: durin42, #hg-reviewers, dlax
Cc: dlax, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D7384: commands: necessary annotations and suppresssions to pass pytype

mharbison72 (Matt Harbison)
In reply to this post by mharbison72 (Matt Harbison)
durin42 updated this revision to Diff 18171.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D7384?vs=18162&id=18171

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7384/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7384

AFFECTED FILES
  mercurial/commands.py

CHANGE DETAILS

diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -72,6 +72,15 @@
     stringutil,
 )
 
+if not globals():
+    from typing import (
+        Any,
+        Dict,
+    )
+
+    for t in (Any, Dict):
+        assert t
+
 table = {}
 table.update(debugcommandsmod.command._table)
 
@@ -2967,7 +2976,7 @@
     if opts.get(b'base'):
         basectx = scmutil.revsingle(repo, opts[b'base'], None)
     # a dict of data to be stored in state file
-    statedata = {}
+    statedata = {}  # type: Dict[bytes, Any]
     # list of new nodes created by ongoing graft
     statedata[b'newnodes'] = []
 
@@ -3237,7 +3246,8 @@
                 )
             # checking that newnodes exist because old state files won't have it
             elif statedata.get(b'newnodes') is not None:
-                statedata[b'newnodes'].append(node)
+                l = statedata[b'newnodes']
+                l.append(node)  # pytype: disable=attribute-error
 
     # remove state when we complete successfully
     if not opts.get(b'dry_run'):
@@ -4725,6 +4735,10 @@
     if opts.get(b'copies'):
         endrev = None
         if revs:
+            # If we're here, we know revs is a smartset.baseset
+            # because we're not possibly in the linerange mode. Sadly,
+            # pytype isn't convinced, so we have to suppress the
+            # warning about list not having a max() method.
             endrev = revs.max() + 1
         getcopies = scmutil.getcopiesfn(repo, endrev=endrev)
 
@@ -7175,11 +7189,13 @@
         t = []
         if incoming:
             t.append(_(b'1 or more incoming'))
-        o = outgoing.missing
+        o = outgoing.missing  # pytype: disable=attribute-error
         if o:
             t.append(_(b'%d outgoing') % len(o))
         other = dother or sother
-        if b'bookmarks' in other.listkeys(b'namespaces'):
+        if b'bookmarks' in other.listkeys(  # pytype: disable=attribute-error
+            b'namespaces'
+        ):
             counts = bookmarks.summary(repo, other)
             if counts[0] > 0:
                 t.append(_(b'%d incoming bookmarks') % counts[0])



To: durin42, #hg-reviewers, dlax
Cc: dlax, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D7384: commands: necessary annotations and suppresssions to pass pytype

mharbison72 (Matt Harbison)
In reply to this post by mharbison72 (Matt Harbison)
marmoute added a comment.


  What's up on this ? It seemed on a good track, but I don't think it landed. @dlax I think you offer to use a context manager got a warm welcome, I would says, go ahead with it.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7384/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7384

To: durin42, #hg-reviewers, dlax
Cc: marmoute, dlax, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D7384: commands: necessary annotations and suppresssions to pass pytype

mharbison72 (Matt Harbison)
In reply to this post by mharbison72 (Matt Harbison)
dlax added a comment.


  In D7384#118739 <https://phab.mercurial-scm.org/D7384#118739>, @marmoute wrote:
 
  > What's up on this ? It seemed on a good track, but I don't think it landed. @dlax I think you offer to use a context manager got a warm welcome, I would says, go ahead with it.
 
  The context manager got in through D7430 <https://phab.mercurial-scm.org/D7430>.

INLINE COMMENTS

> commands.py:4741
> +            # pytype isn't convinced, so we have to suppress the
> +            # warning about list not having a max() method.
>              endrev = revs.max() + 1

I think this comment should be removed since the `# pytype: disable` got removed in the last version of the diff.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7384/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7384

To: durin42, #hg-reviewers, dlax
Cc: marmoute, dlax, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

D7384: commands: necessary annotations and suppresssions to pass pytype

mharbison72 (Matt Harbison)
In reply to this post by mharbison72 (Matt Harbison)
marmoute added a comment.
marmoute accepted this revision.


  There don't seems to be any remaininf feedback on this.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7384/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7384

To: durin42, #hg-reviewers, dlax, marmoute
Cc: marmoute, dlax, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel