Python Bindings: Improved unit tests, documentation and unification of API #1090
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
apage43
reviewed
Jun 30, 2023
| find . | grep -E "(__pycache__|\.pyc|\.pyo$\)" | xargs rm -rf | ||
|
|
||
| black: |
There was a problem hiding this comment.
since these run in the venv perhaps make them depend on the venv target so the venv gets created when running them if it doesn't exist? (and either rename the dir or the target so that actually gets detected properly) / make sure the venv has these deps?
apage43
approved these changes
Jun 30, 2023
rlancemartin
added a commit
to langchain-ai/langchain
that referenced
this pull request
Jul 4, 2023
Running `GPT4All` per the [docs](https://python.langchain.com/docs/modules/model_io/models/llms/integrations/gpt4all), I see: ``` $ from langchain.llms import GPT4All $ model = GPT4All(model=local_path) $ model("The capital of France is ", max_tokens=10) TypeError: generate() got an unexpected keyword argument 'n_ctx' ``` It appears `n_ctx` is [no longer a supported param](https://docs.gpt4all.io/gpt4all_python.html#gpt4all.gpt4all.GPT4All.generate) in the GPT4All API from nomic-ai/gpt4all#1090. It now uses `max_tokens`, so I set this. And I also set other defaults used in GPT4All client [here](https://github.com/nomic-ai/gpt4all/blob/main/gpt4all-bindings/python/gpt4all/gpt4all.py). Confirm it now works: ``` $ from langchain.llms import GPT4All $ model = GPT4All(model=local_path) $ model("The capital of France is ", max_tokens=10) < Model logging > "....Paris." ``` --------- Co-authored-by: R. Lance Martin <rlm@Rs-MacBook-Pro.local>
10 tasks
|
We should add unit test for LangChain in GPT4All. Also, is a backwards compatible fix advised? My PR will force LangChain users to upgrade GPT4All. I welcome an alternative fix for users that upgrade LangChain, but do not have the upgraded GPT4All clinet. |
|
I think this is right
…On Tue, Jul 4, 2023, 12:52 PM Lance Martin ***@***.***> wrote:
We should add unit test for LangChain in GPT4All.
Also, is a backwards compatible fix advised? My PR
<langchain-ai/langchain#7093> will force LangChain
users to upgrade GPT4All. I welcome an alternative fix for users that
upgrade LangChain, but do not have the upgraded GPT4All clinet.
—
Reply to this email directly, view it on GitHub
<#1090 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADJ4TBUZRHRZQ424QCQT5YLXORC6DANCNFSM6AAAAAAZXXD7SM>
.
You are receiving this because you modified the open/close state.Message
ID: ***@***.***>
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Testing, documenting and refactoring the python bindings to: