Repository navigation
stubtest script to compare imported module and stub - #3036
Conversation
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.
unittest.mock problem related to https://bugs.python.org/issue25532
also changed dump['attrs'] to a dict
doing this automatically depends on module stdlib_list, which is not suitable for master branch.
|
Please don't squash, it makes review harder. We always squash when we merge.
|
pkch
left a comment
There was a problem hiding this comment.
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.
| dump_simple(p.default)) | ||
| for name, p in params], | ||
| 'args': [(name, param_kind(p), dump_simple(p.default)) | ||
| for name, p in params], |
There was a problem hiding this comment.
No change here, so can keep the original format to reduce diffs?
| custom_typeshed_dir=None) | ||
| sources = find_modules_recursive(id, lib_path) | ||
| if not sources: | ||
| sys.exit('Error: Cannot find module {}'.format(repr(id))) |
There was a problem hiding this comment.
Why is it no longer necessary to check for empty sources?
| instance_children = defaultdict(lambda: None, instance['names']) | ||
|
|
||
| # TODO: I would rather not filter public children here. | ||
| # For example, what if the checkersurfaces an inconsistency |
|
|
||
| # TODO: I would rather not filter public children here. | ||
| # For example, what if the checkersurfaces an inconsistency | ||
| # in the typing of a private child |
There was a problem hiding this comment.
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?
| result = {} | ||
| for name, value in m.__dict__.items(): | ||
| # Filter out some useless attributes. | ||
|
|
| _, line = inspect.getsourcelines(getattr(m, name)) | ||
| except (TypeError, OSError): | ||
| line = None | ||
|
|
| line = None | ||
|
|
||
| result[name]['line'] = line | ||
|
|
|
|
||
| messages = { | ||
| 'not_in_runtime': ('{error.stub_type} "{error.name}" defined at line ' | ||
| ' {error.line} in stub but is not defined at runtime'), |
There was a problem hiding this comment.
If line is None, you might want to suppress output of at line None
| # TODO: can we filter only call? | ||
| } | ||
|
|
||
| messages = { |
There was a problem hiding this comment.
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.
|
|
||
| @verify.register(nodes.TypeVarExpr) | ||
| def verify_typevarexpr(node, module_node): | ||
| if False: |
There was a problem hiding this comment.
Why do you need if False: yield None here and in several other places? Why not just pass?
|
This script doesn't seem to affect anything except dumpmodule, so I'll just merge this now and we'll can iterate later. |
Code from the sprint this weekend.
There are a few things I'd like to finish up
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