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

feat: custom roots #100

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

backwardspy
Copy link

i'm working with a monorepo, and i can't use a multi-root workspace as it causes vscode to slow to a crawl for some reason (can be 30+ seconds delay on completions, linters, et cetera.) i suspect it may be due to the remote devcontainer workspaces we use. that issue aside, i personally find multi-root workspaces to be a little unwieldy and difficult to manage, getting worse as more roots are added.

i saw #82 and thought that might work for my needs, so i had a go at a prototype implementation. right now it only exists as part of the "recheck" command to make it easier to test, and i wasn't too confident about how to properly integrate it with the extension to start automatically and properly handle changes to the workspace, settings, et cetera.

by way of a demo, i set up a structure to mimic the monorepo i use with some intentional type errors in each of the packages:

.
├── README.md
└── src
    ├── package1
    │   ├── package1
    │   │   └── __init__.py
    │   ├── pyproject.toml
    │   ├── README.md
    │   └── tests
    │       └── __init__.py
    ├── package2
    │   ├── package2
    │   │   └── __init__.py
    │   ├── pyproject.toml
    │   ├── README.md
    │   └── tests
    │       └── __init__.py
    └── package3
        ├── package3
        │   └── __init__.py
        ├── pyproject.toml
        ├── README.md
        └── tests
            └── __init__.py

i configure the extension like so:

{
    "mypy.roots": [
        "src/package1",
        "src/package2",
        "src/package3",
    ]
}

starting the extension in this workspace normally raises the following issue:

[1] Mypy output:
Daemon started
src/package2/tests/__init__.py: error: Duplicate module named "tests" (also at "./src/package1/tests/__init__.py")
src/package2/tests/__init__.py: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#mapping-file-paths-to-modules for more info
src/package2/tests/__init__.py: note: Common resolutions include: a) using `--exclude` to avoid checking one of them, b) adding `__init__.py` somewhere, c) using `--explicit-package-bases` or adjusting MYPYPATH

i then run Mypy: Recheck Workspace to trigger the code i added, and i see the following:

[4] Mypy output:
Daemon started
package3/__init__.py:6:4:6:6: error: Argument 1 to "p3" has incompatible type "int"; expected "str"  [arg-type]
tests/__init__.py:4:4:4:6: error: Argument 1 to "p3" has incompatible type "int"; expected "str"  [arg-type]

[2] Mypy output:
Daemon started
package1/__init__.py:6:4:6:6: error: Argument 1 to "p1" has incompatible type "int"; expected "str"  [arg-type]
tests/__init__.py:4:4:4:6: error: Argument 1 to "p1" has incompatible type "int"; expected "str"  [arg-type]

[3] Mypy output:
Daemon started
package2/__init__.py:6:4:6:6: error: Argument 1 to "p2" has incompatible type "int"; expected "str"  [arg-type]
tests/__init__.py:4:4:4:6: error: Argument 1 to "p2" has incompatible type "int"; expected "str"  [arg-type]

image

at its most basic level, this is working the way i want it to.

if you like the way this is going, i would really appreciate any assistance you can offer towards getting this worked into a proper functioning feature! i am very inexperienced in both typescript and vscode extension development, so i could use all the help i can get.

p.s. sorry for the excess changes, vscode was very set on reformatting everything constantly. if it's annoying i'll edit the commits to remove any non-functional changes.

thanks!

@matangover
Copy link
Owner

Hi! Good start for the implementation. Do you use a different venv for each of your 'roots'? Otherwise, maybe just using mypy.targets could work for you? Are you sure you need a separate daemon for each folder?

@backwardspy
Copy link
Author

we do, though we have been working on ways to build a top-level venv that contains all the dependencies of every project in the repo. i'm not certain this would fully solve the issue though as mypy still detects the "duplicated" test modules:

src/package2/tests/__init__.py: error: Duplicate module named "tests" (also at "./src/package1/tests/__init__.py")

we can exclude tests entirely, but it'd be nice to have them type-checked too.

do you know if this particular issue is solvable with mypy.targets? my current understanding is that a single mypy instance expects a single python package (with any number of unique modules within) to check.

@matangover
Copy link
Owner

i'm not certain this would fully solve the issue though as mypy still detects the "duplicated" test modules

Try giving the 'src' directory as your target. (Instead of giving src/package1 and src/package2)
Then you wouldn't have a collision with 'tests' because the module name would be 'package2.tests'.

my current understanding is that a single mypy instance expects a single python package (with any number of unique modules within) to check.

Technically mypy can process any number of targets (files/folders) in a single run, and they don't need to be from the same package. But yeah you may have issues with module name collisions like you said...

@matangover
Copy link
Owner

In any case I'm not against this PR - don't get me wrong. If you want to complete it you need to make sure all the daemons are stopped properly, as well as handling configuration changes like you said. And thinking how to configure the venv (or other settings?) of each such 'root' separately? (maybe just with the mypy config file in that case?)

@backwardspy
Copy link
Author

yeah i understand - if there is another solution that doesn't need the overhead of multiple mypy processes i'd certainly prefer it! i'll try your suggestion with the targets again tomorrow and see if i can get it working that way. thanks :)

mstmb-alan added a commit to alan-eu/mypy-vscode-plus that referenced this pull request Nov 12, 2024
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