This is an archive of the discontinued Mercurial Phabricator instance.

Differential D7384

commands: necessary annotations and suppresssions to pass pytype
Needs RevisionPublic

Authored by durin42 on Nov 13 2019, 10:51 PM.

Details

Reviewers
dlax
marmoute
baymax
Group Reviewers
hg-reviewers
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.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

durin42 created this revision.Nov 13 2019, 10:51 PM
dlax added a subscriber: dlax.Nov 14 2019, 4:45 AM
dlax added inline comments.
mercurial/commands.py
1124

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.)

durin42 added inline comments.Nov 14 2019, 4:15 PM
mercurial/commands.py
1124

By all means!

dlax requested changes to this revision.Nov 15 2019, 6:08 AM

D7430 makes this changes unnecessary I think.

This revision now requires changes to proceed.Nov 15 2019, 6:08 AM
durin42 marked an inline comment as done.Nov 15 2019, 12:23 PM

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

durin42 updated this revision to Diff 18162.Nov 15 2019, 12:31 PM
dlax added inline comments.Nov 15 2019, 12:40 PM
mercurial/commands.py
4742

revs is always a smartset.baseset per af9c73f26371 so there should be no attribute error. Or is it because logcmdutil.getlinerangerevs() has no type annotation (whereas logcmdutil.getrevs() has some)?

durin42 updated this revision to Diff 18171.Nov 15 2019, 4:55 PM
marmoute added a subscriber: marmoute.Jan 30 2020, 1:32 PM

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.

dlax added a comment.Jan 30 2020, 2:26 PM
In 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.

mercurial/commands.py
4741

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

marmoute accepted this revision.Feb 14 2020, 3:21 AM

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

baymax requested changes to this revision.Jun 8 2020, 1:34 PM

There seems to have been no activities on this Diff for the past 3 Months.

By policy, we are automatically moving it out of the need-review state.

Please, move it back to need-review without hesitation if this diff should still be discussed.

:baymax:need-review-idle:

This revision now requires changes to proceed.Jun 8 2020, 1:34 PM

Revision Contents
Changeset List

PathPackages
Mmercurial/commands.py (26 lines)

Diff 18162

mercurial/commands.py

verify as verifymod, verify as verifymod,
wireprotoserver, wireprotoserver,
) )
from .utils import ( from .utils import (
dateutil, dateutil,
stringutil, stringutil,
) )
if not globals():
from typing import (
Any,
Dict,
)
for t in (Any, Dict):
assert t
table = {} table = {}
table.update(debugcommandsmod.command._table) table.update(debugcommandsmod.command._table)
command = registrar.command(table) command = registrar.command(table)
INTENT_READONLY = registrar.INTENT_READONLY INTENT_READONLY = registrar.INTENT_READONLY
# common command options # common command options
mayupdate(repo, node, show_stats=False) mayupdate(repo, node, show_stats=False)
hbisect.printresult(ui, repo, state, displayer, nodes, bgood) hbisect.printresult(ui, repo, state, displayer, nodes, bgood)
return return
hbisect.checkstate(state) hbisect.checkstate(state)
# actually bisect # actually bisect
nodes, changesets, good = hbisect.bisect(repo, state) nodes, changesets, good = hbisect.bisect(repo, state)
if extend: if extend:
dlaxUnsubmitted
Done

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.)

dlax: This one is sad. I think this can be sorted out by replacing the `try:/finally:` with a context…
durin42AuthorUnsubmitted
Done

By all means!

durin42: By all means!
if not changesets: if not changesets:
extendnode = hbisect.extendrange(repo, state, nodes, good) extendnode = hbisect.extendrange(repo, state, nodes, good)
if extendnode is not None: if extendnode is not None:
ui.write( ui.write(
_(b"Extending search to changeset %d:%s\n") _(b"Extending search to changeset %d:%s\n")
% (extendnode.rev(), extendnode) % (extendnode.rev(), extendnode)
) )
state[b'current'] = [extendnode.node()] state[b'current'] = [extendnode.node()]
) )
revs = list(revs) revs = list(revs)
revs.extend(opts.get(b'rev')) revs.extend(opts.get(b'rev'))
basectx = None basectx = None
if opts.get(b'base'): if opts.get(b'base'):
basectx = scmutil.revsingle(repo, opts[b'base'], None) basectx = scmutil.revsingle(repo, opts[b'base'], None)
# a dict of data to be stored in state file # a dict of data to be stored in state file
statedata = {} statedata = {} # type: Dict[bytes, Any]
# list of new nodes created by ongoing graft # list of new nodes created by ongoing graft
statedata[b'newnodes'] = [] statedata[b'newnodes'] = []
if opts.get(b'user') and opts.get(b'currentuser'): if opts.get(b'user') and opts.get(b'currentuser'):
raise error.Abort(_(b'--user and --currentuser are mutually exclusive')) raise error.Abort(_(b'--user and --currentuser are mutually exclusive'))
if opts.get(b'date') and opts.get(b'currentdate'): if opts.get(b'date') and opts.get(b'currentdate'):
raise error.Abort(_(b'--date and --currentdate are mutually exclusive')) raise error.Abort(_(b'--date and --currentdate are mutually exclusive'))
if not opts.get(b'user') and opts.get(b'currentuser'): if not opts.get(b'user') and opts.get(b'currentuser'):
) )
if node is None: if node is None:
ui.warn( ui.warn(
_(b'note: graft of %d:%s created no changes to commit\n') _(b'note: graft of %d:%s created no changes to commit\n')
% (ctx.rev(), ctx) % (ctx.rev(), ctx)
) )
# checking that newnodes exist because old state files won't have it # checking that newnodes exist because old state files won't have it
elif statedata.get(b'newnodes') is not None: 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 # remove state when we complete successfully
if not opts.get(b'dry_run'): if not opts.get(b'dry_run'):
graftstate.delete() graftstate.delete()
return 0 return 0
# TODO: should follow file history from logcmdutil._initialrevs(), # TODO: should follow file history from logcmdutil._initialrevs(),
# then filter the result by logcmdutil._makerevset() and --limit # then filter the result by logcmdutil._makerevset() and --limit
revs, differ = logcmdutil.getlinerangerevs(repo, revs, opts) revs, differ = logcmdutil.getlinerangerevs(repo, revs, opts)
getcopies = None getcopies = None
if opts.get(b'copies'): if opts.get(b'copies'):
endrev = None endrev = None
if revs: 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.
dlaxUnsubmitted
Not Done

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

dlax: I think this comment should be removed since the `# pytype: disable` got removed in the last…
endrev = revs.max() + 1 # pytype: disable=attribute-error
dlaxUnsubmitted
Not Done

revs is always a smartset.baseset per af9c73f26371 so there should be no attribute error. Or is it because logcmdutil.getlinerangerevs() has no type annotation (whereas logcmdutil.getrevs() has some)?

dlax: `revs` is always a `smartset.baseset` per af9c73f26371 so there should be no attribute error.
getcopies = scmutil.getcopiesfn(repo, endrev=endrev) getcopies = scmutil.getcopiesfn(repo, endrev=endrev)
ui.pager(b'log') ui.pager(b'log')
displayer = logcmdutil.changesetdisplayer( displayer = logcmdutil.changesetdisplayer(
ui, repo, opts, differ, buffered=True ui, repo, opts, differ, buffered=True
) )
if opts.get(b'graph'): if opts.get(b'graph'):
displayfn = logcmdutil.displaygraphrevs displayfn = logcmdutil.displaygraphrevs
dest, dbranch, dother, outgoing = getoutgoing() dest, dbranch, dother, outgoing = getoutgoing()
else: else:
dest = dbranch = dother = outgoing = None dest = dbranch = dother = outgoing = None
if opts.get(b'remote'): if opts.get(b'remote'):
t = [] t = []
if incoming: if incoming:
t.append(_(b'1 or more incoming')) t.append(_(b'1 or more incoming'))
o = outgoing.missing o = outgoing.missing # pytype: disable=attribute-error
if o: if o:
t.append(_(b'%d outgoing') % len(o)) t.append(_(b'%d outgoing') % len(o))
other = dother or sother 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) counts = bookmarks.summary(repo, other)
if counts[0] > 0: if counts[0] > 0:
t.append(_(b'%d incoming bookmarks') % counts[0]) t.append(_(b'%d incoming bookmarks') % counts[0])
if counts[1] > 0: if counts[1] > 0:
t.append(_(b'%d outgoing bookmarks') % counts[1]) t.append(_(b'%d outgoing bookmarks') % counts[1])
if t: if t:
# i18n: column positioning for "hg summary" # i18n: column positioning for "hg summary"