-
Notifications
You must be signed in to change notification settings - Fork 180
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
Fix: User script import now respects script_dir
#1542
base: main
Are you sure you want to change the base?
Conversation
…le_location` if not found.
…from_file_location` if not found.
@microsoft-github-policy-service agree company="Microsoft" |
could you provide an example scenario where the current implementation doesn't work? It was a design decision that the |
See below
By "design" you mean this doc, right? Custom Script
This is the origin implementation def import_module_from_file(module_path: Union[Path, str], module_name: Optional[str] = None):
module_path = Path(module_path).resolve()
if not module_path.exists():
raise ValueError(f"{module_path} doesn't exist")
... It seems this implementation specifically requires module_path (aka user_script) either an absolute path or locates in For example, a project structured like
workflow.json {
"user_script": "file.py",
"script_dir": "my_modules",
} Executing below commands will cause
|
What's more, IMHO, I think maybe Do you think this makes any sense? @jambayk |
Prefer
find_spec
for user script imports; fallback tospec_from_file_location
if not found.Describe your changes
The original implementation of
import_module_from_file
incorrectlychecked for the existence of user scripts in the current working
directory (
cwd
), regardless of thescript_dir
argument provided inimport_user_module
.This PR corrects this behavior by implementing the following logic:
importlib.util.find_spec
.importlib.util.spec_from_file_location
iffind_spec
failsChecklist before requesting a review
lintrunner -a
(Optional) Issue link
#1540