Skip to content

Optional dependencies#7

Merged
andreibondarev merged 7 commits into
patterns-ai-core:mainfrom
mission-met:optional-dependency
May 15, 2023
Merged

Optional dependencies#7
andreibondarev merged 7 commits into
patterns-ai-core:mainfrom
mission-met:optional-dependency

Conversation

@rickychilcott

Copy link
Copy Markdown
Contributor

This PR provides a helper to "require" the gem into the project... but only if it's in already in the Gemfile, otherwise it raises a semi-helpful error message.

In order for this to work, the dependency still needs to be a development dependency (i.e. spec.add_development_dependency) in the gemspec

Works on #3

@rickychilcott

Copy link
Copy Markdown
Contributor Author

@andreibondarev check this out. It's kinda cool. I think we could auto-require all development dependencies in the test environment, but there are pros and cons to that. Let me know if you think we should.

@andreibondarev

Copy link
Copy Markdown
Collaborator

@andreibondarev check this out. It's kinda cool. I think we could auto-require all development dependencies in the test environment, but there are pros and cons to that. Let me know if you think we should.

I think that is okay, yes.

Comment thread lib/llm/cohere.rb Outdated
Comment thread lib/optional_dependency_helper.rb Outdated
Comment thread lib/optional_dependency_helper.rb Outdated
Comment thread langchain.gemspec
spec.require_paths = ["lib"]

spec.add_development_dependency "pry-byebug", "~> 3.10.0"
spec.add_development_dependency "cohere-ruby", "~> 0.9.3"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think we should move ALL of the gems here or maybe leave some core ones under actual deps below?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure. Only the LLMs would feel core enough, but then again, why have cohere if you're using openai?

I could imagine actual core ones being activesupport or other similar gems needed for the plumbing of langchain.

So yes, but in its current state, there might not be any true dependences at all.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea is that you can swap out different LLMs. Ideally, eventually, I think we ought to have a lot more LLM options to pick from.

@andreibondarev andreibondarev left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rickychilcott Left you some feedback! THANK YOU for the contribution so far!

@andreibondarev andreibondarev mentioned this pull request May 15, 2023
@rickychilcott

Copy link
Copy Markdown
Contributor Author

I ended up calling the method depend_on, and it felt like the best name at this time. Also, I was unable to figure out how to "auto require" each of the gems in test because there can be a divergence between the gem name and the require statement. So those spec files that need to reference the underlying library will need to manually require.

I think this is ready for another review and a thumbs up if we're on the same page to apply this everywhere.

@andreibondarev

Copy link
Copy Markdown
Collaborator

@rickychilcott Would you please upmerge and resolve the conflict (Gemfile.lock I think) and I'll merge this PR in! Thanks again!

@rickychilcott

Copy link
Copy Markdown
Contributor Author

Done!

@andreibondarev andreibondarev merged commit 1d12f7a into patterns-ai-core:main May 15, 2023
@rickychilcott

Copy link
Copy Markdown
Contributor Author

Cool! I'm happy to apply this pattern across the Readme and codebase in a separate PR. I might get an opportunity to do so later today or tomorrow.

@andreibondarev

Copy link
Copy Markdown
Collaborator

@rickychilcott Let's do it! I think the examples in examples/ might need to be updated as well. Or at least mention like:

# gem install "cohere-ruby"

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants