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

Remove Use Of import_optional_dependency #1470

Open
1 task done
collindutter opened this issue Dec 19, 2024 · 2 comments
Open
1 task done

Remove Use Of import_optional_dependency #1470

collindutter opened this issue Dec 19, 2024 · 2 comments
Labels
type: discussion Open for conversation or brainstorming type:enhancement Improvements to existing features
Milestone

Comments

@collindutter
Copy link
Member

Is your feature request related to a problem? Please describe.
import_optional_dependency was introduced in order to safely import optional dependencies while raising a helpful error message. Unfortunately this function removes the type hints on imports, and requires weird import patterns.

Describe the solution you'd like
Refactor the optional import dependency mechanism to use more native python functionality. I.e.

try:
  import anthropic
catch ImportError:
  raise "You need to install the extra for anthropic"

Or

def create_missing_extra_class(message):
    class MissingExtra:
        def __init__(self):
            raise Exception(message)
    return MissingExtra

if is_dependency_installed("anthropic"):
    from .prompt.anthropic_prompt_driver import AnthropicPromptDriver
else:
    AnthropicPromptDriver = create_missing_extra_class("You missed the anthropic-whatever extra!")


__all__ = ['AnthropicPromptDriver']

Describe alternatives you've considered
Adding type hint support to import_optional_dependency. Doesn't seem possible, and is still clunky to use.

@collindutter collindutter added the type:enhancement Improvements to existing features label Dec 19, 2024
@collindutter collindutter added this to the 2.0 milestone Dec 19, 2024
@vachillo
Copy link
Member

it seems like import_optional_dependency is just a convenience around the first option no? if we can get type hints to work as expected, we should keep it around

@collindutter collindutter added the type: discussion Open for conversation or brainstorming label Dec 20, 2024
@collindutter
Copy link
Member Author

Roughly, but the larger issue is how we use import_optional_dependency. We have all imports done lazily rather than eagerly at the top of the file. I think my ideal is something like:

if is_dependency_installed("anthropic"):
    from anthropic import Client
else:
    raise MissingExtraError("drivers-anthropic-prompt")

Feels very un-magical.

The challenge with moving away from lazy imports is we'd have to update our import scheme to something like:

from griptape.drivers.prompt.anthropic import AnthropicPromptDriver

If we wanted to keep the from griptape.drivers import AnthropicPromptDriver syntax we'd need to go down the path of creating the dummy classes that fail at runtime.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: discussion Open for conversation or brainstorming type:enhancement Improvements to existing features
Projects
None yet
Development

No branches or pull requests

2 participants