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] Use meta path instead of AST transform to implement new config. #1215

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

mzr1996
Copy link
Member

@mzr1996 mzr1996 commented Jun 28, 2023

Motivation

In the original new config implementation, we use AST transform to manipulate the import statements.
It's a little obscure, and hard to debug.

Modification

Python provide a native feature to override the import behaviors. In this PR, we use sys.meta_path to
implement the lazy import and base config file import.

BC-breaking

I have tested on MMPreTrain new config, it shouldn't change the original usages, but please check it carefully.

Checklist

  1. Pre-commit or other linting tools are used to fix the potential lint issues.
  2. The modification is covered by complete unit tests. If not, please add more unit test to ensure the correctness.
  3. If the modification has potential influence on downstream projects, this PR should be tested with downstream projects, like MMDet or MMCls.
  4. The documentation has been modified accordingly, like docstring or example tutorials.

@mzr1996 mzr1996 force-pushed the meta-path-config branch 3 times, most recently from 3596af5 to 5646bf0 Compare July 18, 2023 01:59
@mzr1996 mzr1996 force-pushed the meta-path-config branch 2 times, most recently from 4364d52 to d98b733 Compare August 24, 2023 07:11
mmengine/config/config.py Show resolved Hide resolved
mmengine/config/lazy.py Outdated Show resolved Hide resolved
mmengine/config/lazy.py Outdated Show resolved Hide resolved
mmengine/config/lazy.py Outdated Show resolved Hide resolved
mmengine/config/lazy.py Outdated Show resolved Hide resolved
mmengine/config/new_config.py Show resolved Hide resolved
mmengine/config/new_config.py Show resolved Hide resolved
mmengine/config/new_config.py Show resolved Hide resolved
mmengine/config/new_config.py Show resolved Hide resolved
mmengine/config/new_config.py Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants