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

For autogen-ext sub modules, make sure classes can be imported independently without getting import errors from missing dependencies in other modules #4408

Open
ekzhu opened this issue Nov 27, 2024 · 2 comments · May be fixed by #4410
Milestone

Comments

@ekzhu
Copy link
Collaborator

ekzhu commented Nov 27, 2024

Right now the directory structure is not great for submodule import:

autogen_ext
- code_executors
  __init__.py
  _azure_container_code_executor.py
  _docker_code_executor.py
- agents
  __init__.py # The init file contains all the imports from sub modules, causing import errors

It should be:

autogen_ext
- code_executors
  - __init__.py
  - azure
    - __init__.py
    _azure_container_code_executor.py
  - docker
    - __init__.py 
    - _docker_code_executor.py
- agents
  - __init__.py 
  - web_surfer
    - __init__.py
    - _multi_modal_web_surfer.py
@ekzhu ekzhu added this to the 0.4.0 milestone Nov 27, 2024
@ekzhu ekzhu linked a pull request Nov 27, 2024 that will close this issue
@lspinheiro
Copy link
Collaborator

@ekzhu , I think the pattern that @jackgerrits adopted for code executors and tools was to move the imports inside the classes and throw a runtime error if the dependencies are missing. I did something similar in #4423. The main difference is that I still kept the imports in the top of the module but sued a module variable to track whether the dependencies are available and delay the exception to runtime. The only drawback is having some imports done twice because pyright complains the varaibles may be unbound (even with the variable check in the code).

If that pattern is acceptable then I can update the models and web surfer in the same way.

BTW, happy thanksgiving!

@ekzhu
Copy link
Collaborator Author

ekzhu commented Nov 29, 2024

I think previously we want to export classes from submodule directly because the API documentation site shows a nested structure so it was easier to navigate the docs. However we have found that a flattened structure is better, see the issue #4377. Once the structure is flattened, having the full submodule path for class is actually better because now the public utils related to the classes are grouped together with the class as well.

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

Successfully merging a pull request may close this issue.

3 participants