Skip to content

Various fixes to following imports in mypy daemon - #8590

Merged
JukkaL merged 9 commits into
masterfrom
daemon-fixes-2
Mar 30, 2020
Merged

JukkaL merged 9 commits into
masterfrom
daemon-fixes-2

Conversation

@JukkaL

@JukkaL JukkaL commented Mar 25, 2020

Copy link
Copy Markdown
Collaborator

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

JukkaL commented Mar 27, 2020

Copy link
Copy Markdown
Collaborator Author

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

@msullivan msullivan left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good

Comment thread mypy/dmypy_server.py

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
Copy Markdown
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
Copy Markdown
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