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

update ollama code to get model.model property instead of model.name #2188

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

James-Pickett
Copy link

@James-Pickett James-Pickett commented Feb 2, 2025

Changes started with this comment:

#2030 (comment)

Thank you @SailorJoe6

Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Fixes # (issue)

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

Running locally with print statements.

Then running the tests in

  • tests/embeddings/test_ollama_embeddings.py
  • tests/llms/test_ollama.py

Sorry, I don't have time to figure out how to run full test suite.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • My changes generate no new warnings
  • New and existing unit tests pass locally with my changes (at least in files mentioned above)
  • Any dependent changes have been merged and published in downstream modules
  • I have checked my code and corrected any misspellings

Maintainer Checklist

  • closes #xxxx (Replace xxxx with the GitHub issue number)
  • Made sure Checks passed

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


jaypickle seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@James-Pickett James-Pickett marked this pull request as ready for review February 2, 2025 19:30
@James-Pickett James-Pickett changed the title update ollama code go model model property instead of name update ollama code to get model.model property instead of model.name Feb 2, 2025
@entelligence-ai-reviews
Copy link

Walkthrough

The PR updates the key from name to model in several files to ensure correct model retrieval. Changes are minor and consistent across files.

Entelligence.ai can learn from your feedback. Simply add 👍 / 👎 emojis to teach it your preferences. More shortcuts below

Emoji Descriptions:

  • ⚠️ Potential Issue - May require further investigation.
  • 🔒 Security Vulnerability - Fix to ensure system safety.
  • 💻 Code Improvement - Suggestions to enhance code quality.
  • 🔨 Refactor Suggestion - Recommendations for restructuring code.
  • ℹ️ Others - General comments and information.

Interact with the Bot:

  • Send a message or request using the format:
    @bot + *your message*
Example: @bot Can you suggest improvements for this code?
  • Help the Bot learn by providing feedback on its responses.
    @bot + *feedback*
Example: @bot Do not comment on `save_auth` function !

@Dev-Khant
Copy link
Member

Hey @James-Pickett Can you please check why tests are failing?

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.

4 participants