-
-
Notifications
You must be signed in to change notification settings - Fork 3k
Add experimental hidden flag to tweak dependencies #5367
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
JukkaL
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks mostly good, just a few minor things.
| self.add_dependency(make_trigger(o.func.fullname())) | ||
| if self.options is not None and self.options.tweaked_deps: | ||
| for d in o.decorators: | ||
| if isinstance(d, RefExpr) and d.fullname is not None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also look for things like @foo(...) (i.e. call expressions).
mypy/server/deps.py
Outdated
| if self.options is not None and self.options.tweaked_deps: | ||
| for d in o.decorators: | ||
| if isinstance(d, RefExpr) and d.fullname is not None: | ||
| self.add_dependency(make_trigger(d.fullname), make_trigger(o.func.fullname())) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add comment explaining why we want to have this.
| for base_info in non_trivial_bases(info): | ||
| for name, node in base_info.names.items(): | ||
| if self.options and self.options.tweaked_deps: | ||
| if name not in info.names: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add comment describing why we want to do this (with an example perhaps).
| for trigger in get_type_triggers(o.type): | ||
| self.add_dependency(trigger) | ||
| if self.options and self.options.tweaked_deps: | ||
| if (isinstance(rvalue, CallExpr) and isinstance(rvalue.callee, RefExpr) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add comment explaining why we do this.
mypy/main.py
Outdated
| # --local-partial-types disallows partial types spanning module top level and a function | ||
| # (implicitly defined in fine-grained incremental mode) | ||
| parser.add_argument('--local-partial-types', action='store_true', help=argparse.SUPPRESS) | ||
| # --tweaked-deps adds some more dependencies that are not semantically needed, but |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mention that this is an experimental, undocumented feature that may go away or be changed at any time.
mypy/main.py
Outdated
| # may be helpful to determine relative importance of classes and functions for overall | ||
| # type precision in a code base. It also _removes_ some deps, so this flag should be never | ||
| # used except for generating code stats. | ||
| parser.add_argument('--tweaked-deps', action='store_true', help=argparse.SUPPRESS) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe rename to something like --logical-deps or --semantic-deps and make this imply --cache-fine-grained? The motivation is that this makes the dependencies more independent of the details of how fine-grained incremental checking works.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I went with --logical-deps it sounds better.
JukkaL
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the updates, looks good now. Feel free to merge once you've checked that this works against internal Dropbox codebases.
This flag will tweak fine grained dependencies to reflect propagation of type (im)precision instead of actual semantic dependencies. The flag should never be used to run actual type check, only to analyze type coverage in a code base.
The flag could be useful when annotating legacy code, to check which functions (decorators, base classes, variables) are most important to annotate first.