Skip to content

Daemon support for --follow-imports=normal #5870

Closed

Description

@msullivan

Currently the daemon only supports errors and ignore, which is a big limitation.
Additionally, while we reject follow-imports=normal as a top-level option, we
silently allow it for module-level definitions while not correctly implementing it.
(Including for all stubs, as in #5566.)

Activity

  1. self-assigned this
    on Nov 5, 2018
  2. msullivan commented on Nov 6, 2018

    @msullivan
    CollaboratorAuthor

    The main difficulty in supporting import following comes from detecting when a module that is imported but where the error for the missing import is suppressed. For example if we have:

    import foo  # type: ignore
    foo.bar(10)
    

    and we fail to find foo, we will give everything in it an Any type and carry on. If in a subsequent run a foo.py or foo.pyi (or foo/__init__.py or foo/__init__.pyi... and some -stubs directories) is added somewhere on mypy's search path, we need to process it and then recheck any uses of foo.

    To make things worse, files that do exist can get preempted by another module being found earlier in the search path.

    The problem is that there can be a lot of places that any given module might live. We infer a search path of root directories based on the sources passed in and where __init__.py files live, and if you have ~200 root directories, there are about ~1000 places where each module might be found. We can't watch all of those files.

  3. emmatyping commented on Jan 6, 2019

    @emmatyping
    Member

    if you have ~200 root directories, there are about ~1000 places where each module might be found. We can't watch all of those files.

    If I am understanding things correctly, it sounds to me like we should keep track of changes via something like watchdog? Then we can greatly limit the search path of changes to just those where a file was created? In fact, it should be a lot easier to just filter the file paths watchdog gives us while the daemon is running by the set of search directories, then we know exactly what relevant changes were made. The number of total changes made should be relatively small (in the dozens) instead of huge (in the 1000s).

  4. ilevkivskyi commented on Jan 9, 2019

    @ilevkivskyi
    Member

    @ethanhs the recently added daemon flags --update/--remove allow simple "external" integration with such tools (watchman is another example).

    If we will not find other solution, then we can consider making one of such services a dependency. We can make it an optional dependency, for example dmypy start --follow-imports=error will work as previously, but dmypy start --follow-imports=normal will fail unless run as dmypy start --follow-imports=normal --fswatcher=<service>. Here <service> may be a shell script with minimal API, or maybe choice of supported services for which we know the API like watchdog or watchman.

  5. emmatyping commented on Jan 9, 2019

    @emmatyping
    Member

    @ilevkivskyi ah, then I guess we just need to have a good interface for selecting a backend for this information.

    I like the dmypy start --follow-imports=normal --fswatcher=<service> spelling. I would strongly suggest just calling watchman/watchdog APIs directly, as shell scripts are not particularly cross platform.

  6. matangover commented on Oct 21, 2019

    @matangover
    Contributor

    This limitation is blocking usage of the daemon with PEP 561 packages that use inline type information. However, PEP 561 packages that have type information in pyi files do work (#5566).

    This inconsistency is quite surprising. I can imagine a package moving their type info from pyi to py and suddenly ceasing to work with the daemon.

    Personally, I think package imports should be followed by the daemon. The original concern of tracking updates isn't really relevant here – installed packages rarely change (unlike user code). In case of installing/updating/removing a package, the daemon can be restarted.

    So maybe there should be a follow_imports_for_installed_packages option? Similar to follow_imports_for_stubs, when set to 'False' it will override follow_imports to always use 'normal' for installed packages. I would argue that this should be set to False by default, but then there's backwards compat concerns (?). In any case, it seems like full --follow-imports=normal for the daemon will not be supported anytime soon because it's difficult to implement and isn't a common use case.

    Repro steps:

    $ echo "import hello" > test.py
    
    # Package with inline type information: works with mypy, not with dmypy
    $ pip install "git+https://github.com/ymyzk/pep-561-samples#subdirectory=hello-typed1"
    $ mypy test.py
    Success: no issues found in 1 source file
    $ dmypy run -- --follow-imports=error test.py; dmypy stop
    test.py:1: error: Cannot find module named 'hello'
    
    # Package with type information in stubs: works with both mypy and dmypy
    $ pip uninstall hello-typed1
    $ pip install "git+https://github.com/ymyzk/pep-561-samples#subdirectory=hello-typed2"
    $ mypy test.py
    Success: no issues found in 1 source file
    $ dmypy run -- --follow-imports=error test.py; dmypy stop
    Success: no issues found in 1 source file
    

    If it's useful to anyone, there is a workaround that uses the 'bug' described by @msullivan in the first post – creating mypy.ini with the following contents will allow the daemon to follow imports to package that use inline type info:

    [mypy]
    [mypy-hello]
    follow_imports=normal
  7. matangover commented on Oct 21, 2019

    @matangover
    Contributor

    Another idea: a --follow-imports=once option (supported by daemon only) which is essentially the current functionality of 'follow-imports=normal' but makes it explicit for users that the daemon will not track updates to dependencies that aren't specified in the sources on the command line.

    I can work on a PR if you accept.

  8. msullivan commented on Oct 21, 2019

    @msullivan
    CollaboratorAuthor

    That is a reasonable strategy, I think. I would probably accept a PR to do that.

  9. assigned and unassigned on Apr 1, 2020
  10. added a commit that references this issue on Apr 9, 2020
    7802f37
  11. vjpr commented on Apr 22, 2020

    @vjpr

    FYI: The linked PR has enabled this on master.

    python3 -m pip install -U git+git://github.com/python/mypy.git
    

    @JukkaL Any idea on timeline for this being released?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions