Moved Ollama implementation to ollama.py; removed redundant import in mistral.py; minor linting in both

[andreped]

Just observed this when I was addressing PR #239.

To ensure that everything was OK, I also looked at the Mistral implementation and found a redundant import.
I then linted both using the pre-commit hooks. Feature-wise there should not be any major changes.

With this PR, users should be able to import ollama in the same way as done for mistral:

from vanna.mistral.mistral import Mistral
from vanna.ollama.ollama import Ollama

I believe this import will be slightly different in the current main branch, as the Ollama implementation currently lies in the ollama/__init__.py instead of the ollama/ollama.py file.

[andreped]

@zainhoda Any thoughts on this PR? Looks rather straight forward?

[skoschik]

I've recently been exploring the Vanna Ollama import documentation on the vanna.ai website and noticed that there appears to be a discrepancy between the current implementation details and the documentation provided at https://vanna.ai/docs/postgres-ollama-chromadb/. (probably elsewhere ollama import is mentioned)

The docs suggest
from vanna.ollama import Ollama
but is now
from vanna.ollama.ollama import Ollama

I wasn't sure if the documentation was managed directly within the GitHub repository or maintained separately. If it's on GitHub and contributions are welcome, I'd be happy to help with updating the documentation to reflect the current state of the project.

Thank you for your hard work on this project, and I look forward to your guidance on how best to proceed with this observation.

[zainhoda]

@skoschik thanks for reporting this! So I'd actually prefer to have the format be:

from vanna.ollama import Ollama

IIRC, in src/vanna/ollama/init.py if we import the class, then I think this should work?

[andreped]

IIRC, in src/vanna/ollama/init.py if we import the class, then I think this should work?

Yes, something like this should work. I can make a PR to address this, if you want? :]

[zainhoda]

That would be great!

…

On Fri, Mar 29, 2024 at 1:06 PM André Pedersen ***@***.***> wrote: IIRC, in src/vanna/ollama/*init*.py if we import the class, then I think this should work? Yes, something like this should work. I can make a PR to address this, if you want? :] — Reply to this email directly, view it on GitHub <#259 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABWQVKQ7X3K422SRMBE7BP3Y2WGPPAVCNFSM6AAAAABDW2XALOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMRXGUYDANJYGU> . You are receiving this because you modified the open/close state.Message ID: ***@***.***>

[andreped]

That would be great!

Wouldn't we need to apply the same to other models like Mistral as well?

---

EDIT: Just checked. There seems like most, if not all, model clients have the same issue. I will try to address this in one PR.

[zainhoda]

Yeah if it works for this, then we should probably simplify the imports everywhere (being careful to maintain backwards compatibility). In this specific case, we have users who had a way of importing that is now broken

…

On Fri, Mar 29, 2024 at 1:11 PM André Pedersen ***@***.***> wrote: That would be great! Wouldn't we need to apply the same to other models like Mistral as well? — Reply to this email directly, view it on GitHub <#259 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABWQVKU5D72DJLBIWWAMDOTY2WHB3AVCNFSM6AAAAABDW2XALOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMRXGUYDMNZQGI> . You are receiving this because you modified the open/close state.Message ID: ***@***.***>

[andreped]

Made a PR #321. Let's discuss this further there :]

[andreped]

In this specific case, we have users who had a way of importing that is now broken

No worries. My PR will allow for both types of imports. Same applies to some other submodules. I updated the PR to extend this to other relevant submodules.