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

Improve docs to clarify that import heuristics don't work for local imports #5836

Open
irm-codebase opened this issue May 1, 2024 · 6 comments
Assignees
Labels
bug Something isn't working

Comments

@irm-codebase
Copy link

irm-codebase commented May 1, 2024

As the name says, discovery heuristics does not seem to work for local imports.
There are a bunch of issues marking this as solved... but the current documentation seems to ignore them (#1167, #68, #253, #3592 and lately #5226).

Obviously finding what works and what does not is... quite hard at the moment.

So, should discovery heuristics work for local imports by default? And if so, how to fix it?

Environment data

  • Language Server version: v2024.4.1
  • OS and version: Fedora 39
  • Python version (& distribution if applicable, e.g. Anaconda): python 3.10.13, conda 23.11.0 (using miniforge)

Code Snippet

Nothing too complicated: I am importing a local script

from typing import Optional

import geopandas as gpd
import numpy as np
import pandas as pd
import parse_lombardi  # Local, Pylance fails to detect it
# from src.calliope_pathways.models.italy_stationary.pre_processing import parse_lombardi # works, but ewww

The project structure is

calliope-pathways/
┣ src/
┃ ┣ calliope_pathways/
┃ ┃ ┣ config/
┃ ┃ ┣ math/
┃ ┃ ┣ models/
┃ ┃ ┃ ┣ italy_stationary/
┃ ┃ ┃ ┃ ┣ data_sources/
┃ ┃ ┃ ┃ ┣ model_config/
┃ ┃ ┃ ┃ ┣ pre_processing/
┃ ┃ ┃ ┃ ┃ ┣ parse_lombardi.py  <--- Imported
┃ ┃ ┃ ┃ ┃ ┗ parse_ppm.py  <--- Importing
┃ ┃ ┃ ┗ national_scale/
┃ ┃ ┃   ┣ data_sources/
┃ ┃ ┃   ┣ model_config/
┃ ┃ ┣ __init__.py
┃ ┃ ┣ _version.py
┃ ┃ ┣ models.py
┃ ┃ ┣ py.typed
┃ ┃ ┗ util.py

Adding the path to extraPaths solves it, but this should be automatic with heuristics enabled (which is my case, per default).

{
    "python.analysis.extraPaths": ["src/calliope_pathways/models/italy_stationary/pre_processing"],
    // "python.analysis.logLevel": "Trace"
}

Repro Steps

  1. Create a project, add multiple folders / files
  2. Try to import a local python file wherever.
  3. Fails

Expected behavior

Pylance should resolve local imports by default using heuristics.

Actual behavior

Pylance does not resolve the import using heuristics.

Logs

Logs too big... see attachment.
2024 05 01 15 18 32 444.txt

@github-actions github-actions bot added the needs repro Issue has not been reproduced yet label May 1, 2024
@heejaechang
Copy link
Contributor

heejaechang commented May 1, 2024

it is a dup of #5434

currently it is by design. we don't consider alias (import x or from y import x) in user files (ones you refer as local import) for now due to perf issue. (unlike installed packages that dont change once installed, these local imports, we need to repeatedly re-index all dependent files as user change a file)

we are planning to add full mode for people who wants it (it is opt in since most of people don't want to pay that perf hit) but no specific ETA yet.

@irm-codebase
Copy link
Author

irm-codebase commented May 2, 2024

@heejaechang got it.
Perhaps it would be good to specify this lacking feature in the READ.me or the documentation more clearly?

I think the reason why issues on this topic keep appearing this is that users can only find outdated information at stackoverflow or old comments in this repo. Maybe I did not find the right section, but if it was addressed clearly (i.e., directly stating "we do not support user file/local discovery, do this instead") it would do wonders for clarity.

@luabud luabud changed the title Import heuristics not working for local imports Improve docs to clarify that import heuristics don't work for local imports May 2, 2024
@luabud luabud assigned luabud and unassigned KacieKK May 2, 2024
@luabud luabud added needs documentation Improvements or additions to documentation and removed needs repro Issue has not been reproduced yet labels May 2, 2024
@heejaechang
Copy link
Contributor

heejaechang commented May 7, 2024

@irm-codebase it looks like I misunderstood your issue. so you are asking about unresolve import not add import code action?

@heejaechang heejaechang added bug Something isn't working and removed needs documentation Improvements or additions to documentation labels May 7, 2024
@heejaechang heejaechang assigned heejaechang and unassigned luabud May 7, 2024
@irm-codebase
Copy link
Author

irm-codebase commented May 7, 2024

Correct. Here, parse_lombadi is on the same directory as the script calling it.

image

@heejaechang
Copy link
Contributor

I tried your scenario on both windows, WSL, 'Linux` and all worked for me. I guess we need to improve our logging to see what is going on.

@heejaechang
Copy link
Contributor

I added new logging for this case. it will be included in our next pre-release. we probably need that log (unless we can repro it) to figure out what is going on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants