Skip to content

stubtest script to compare imported module and stub - #3036

Merged
gvanrossum merged 6 commits into
python:masterfrom
jgarst:master
Apr 30, 2017
Merged

gvanrossum merged 6 commits into
python:masterfrom
jgarst:master

Conversation

@jgarst

@jgarst jgarst commented Mar 20, 2017

Copy link
Copy Markdown
Contributor

Code from the sprint this weekend.

There are a few things I'd like to finish up

  • One or two examples of filters, to make the code less verbose. e.g. don't show missing annotations of modules.
  • checking the types of Var nodes
  • checking the arguments and return types of functions
  • a last pass to fix style, make it less scripty.

I think I can finish these up in the evening of this week, but the code is here in case you want to run out ahead, or if I get hit by a bus.

Also let me know if you want the commits squashed, or anything else before merging.
-- Jared

Jared Garst added 5 commits March 19, 2017 21:42
Error defined as NamedTuple(name, error_type, message).
stubtest no longer errors when there is no stubfile
Instead of calling the dumpmodule script, do a relative import and
call the function directly.
also changed dump['attrs'] to a dict
doing this automatically depends on module stdlib_list, which is not
suitable for master branch.
@gvanrossum

gvanrossum commented Mar 20, 2017 via email

Copy link
Copy Markdown
Member

@pkch pkch left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like your refactoring, it looks much cleaner now.

BTW, you might want to use a different branch than master locally for PRs, otherwise if you ever have to update and merge from the official master, it will be impossible.

Comment thread scripts/dumpmodule.py
dump_simple(p.default))
for name, p in params],
'args': [(name, param_kind(p), dump_simple(p.default))
for name, p in params],

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No change here, so can keep the original format to reduce diffs?

Comment thread scripts/stubtest.py
custom_typeshed_dir=None)
sources = find_modules_recursive(id, lib_path)
if not sources:
sys.exit('Error: Cannot find module {}'.format(repr(id)))

@pkch pkch Apr 11, 2017 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is it no longer necessary to check for empty sources?

Comment thread scripts/stubtest.py
instance_children = defaultdict(lambda: None, instance['names'])

# TODO: I would rather not filter public children here.
# For example, what if the checkersurfaces an inconsistency

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

space after 'checker'

Comment thread scripts/stubtest.py

# TODO: I would rather not filter public children here.
# For example, what if the checkersurfaces an inconsistency
# in the typing of a private child

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But if private children are not supposed to be used outside the module, is it worth trying to add them to the stub, especially since some of them may be very tricky to type correctly?

Comment thread scripts/dumpmodule.py
result = {}
for name, value in m.__dict__.items():
# Filter out some useless attributes.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unnecessary empty line

Comment thread scripts/dumpmodule.py
_, line = inspect.getsourcelines(getattr(m, name))
except (TypeError, OSError):
line = None

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unnecessary empty line

Comment thread scripts/dumpmodule.py
line = None

result[name]['line'] = line

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unnecessary empty line

Comment thread scripts/stubtest.py

messages = {
'not_in_runtime': ('{error.stub_type} "{error.name}" defined at line '
' {error.line} in stub but is not defined at runtime'),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If line is None, you might want to suppress output of at line None

Comment thread scripts/stubtest.py
# TODO: can we filter only call?
}

messages = {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would use constants like NOT_IN_RUNTIME = ('{error.stub_type} ...., instead of dictionary; that way IDE auto-completes and interpreter will identify a typo right away.

Comment thread scripts/stubtest.py

@verify.register(nodes.TypeVarExpr)
def verify_typevarexpr(node, module_node):
if False:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you need if False: yield None here and in several other places? Why not just pass?

@gvanrossum

Copy link
Copy Markdown
Member

This script doesn't seem to affect anything except dumpmodule, so I'll just merge this now and we'll can iterate later.

@gvanrossum
gvanrossum merged commit f07c8be into python:master Apr 30, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants