Skip to content

Various fixes to following imports in mypy daemon #8590

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

Merged
merged 9 commits into from
Mar 30, 2020
Merged

Conversation

JukkaL
Copy link
Collaborator

@JukkaL JukkaL commented Mar 25, 2020

Included fixes:

  • filter out bogus suppressed modules to speed things up
  • fix issue with import inside function
  • fix stub packages
  • also support / path separators on Windows
  • update search path based on paths given on the command line
  • support changes to files passed on the command line
  • add logging
  • minimal namespace package support

Namespace package support is not complete, but the issue I found isn't
specific to following imports in mypy daemon, so I didn't try to fix it
in this PR. I'll create a follow-up issue.

@JukkaL JukkaL requested a review from msullivan March 25, 2020 18:59
@JukkaL
Copy link
Collaborator Author

JukkaL commented Mar 27, 2020

The travis build seems to have passed, but it's not reflected here.

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 good

@@ -720,6 +741,9 @@ def pretty_messages(self, messages: List[str], n_sources: int,

def update_sources(self, sources: List[BuildSource]) -> None:
paths = [source.path for source in sources if source.path is not None]
if self.following_imports():
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't follow this really?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If there are directories in paths, we'll get exceptions inside fswatcher.

@JukkaL JukkaL merged commit 828a9eb into master Mar 30, 2020
@JelleZijlstra JelleZijlstra deleted the daemon-fixes-2 branch April 1, 2020 21:23
JukkaL added a commit that referenced this pull request Apr 3, 2020
This includes a bunch of changes to following imports:

* Fix bug with cached modules without ASTs by loading the AST as needed
* Improve docstrings and simplify code
* Rename `follow_imports`
* Merge `seen` and `sources_set`
* Address some TODO items

This may be easier to review by looking at individual commits.

I think that following imports is ready to be enabled after this has been
merged. We can always revert the change before the next release if we
we encounter major issues.

Major things I know about that still don't work (I'll try to address at least the 
first one before the next release):

* `-m` and `-p` don't work correctly with dmypy (I think that happens also
  in other modes, but might be worse when following imports)
* Suppressed modules are incorrect with namespace modules (happens
  also in other modes)
* File events are not supported (but these are undocumented, so it's
  not critical?)
* Performance with a huge number of files is unknown (other import modes
  have better performance and can be used as fallbacks if this is a problem)

Work towards #5870.

Continues progress made in #8590.
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.

2 participants