Skip to content
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

Allow passing path/to/file.py:line instead of fully.qualified.name to dmypy suggest #7483

Merged
merged 10 commits into from
Sep 10, 2019

Conversation

ilevkivskyi
Copy link
Member

This should help with integrations with editors and IDEs, where getting line number of a function might be much easier than the full name.

The implementation in mostly straightforward. I also did a little refactoring to reuse the existing logic efficiently. The only important difference that I introduced after few experiments is that I force reloading of the file before trying to search the function, since even the smallest edit can offset the line number and lead to some weird results.

@ilevkivskyi
Copy link
Member Author

ilevkivskyi commented Sep 8, 2019

Couple observations:

  • This is still a bit slow, takes about 7.5s (interpreted) for a method in mypy/types.py, while providing full name takes 6.5s.
  • Suggestion logic doesn't work for yield.

Copy link
Collaborator

@msullivan msullivan left a comment

Choose a reason for hiding this comment

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

Looks great. This will make this a lot better for interactive type uses.

Is the slowdown mostly caused by the extra reload?
Maybe suggest ought to just do a recheck before running?

@@ -285,13 +287,17 @@ def maybe_suggest(self, step: int, server: Server, src: str) -> List[str]:
try_text=try_text, flex_any=flex_any,
callsites=callsites))
val = res['error'] if 'error' in res else res['out'] + res['err']
if json:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for getting JSON tests working!

modname, tail = target
node = None # type: Optional[SymbolNode]
if ':' in key:
file, line = key.split(':')
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add a count so a bad argument can't crash the daemon?

@ilevkivskyi
Copy link
Member Author

Is the slowdown mostly caused by the extra reload?

It looks like yes, I tried to play with this a bit more, and it looks like penalty is typically a bit lower, around 500ms (interpreted) for mypy self-suggest with some removed signatures. For compiled this would mean something like 1100ms vs 1200ms, so probably not worth it to try to optimize now (it affects only suggest, not other commands). We can come back to this later if it turns out o be important.

@ilevkivskyi ilevkivskyi merged commit a86be75 into python:master Sep 10, 2019
@ilevkivskyi ilevkivskyi deleted the suggest-lineno branch September 10, 2019 17:20
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.

None yet

2 participants