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

Deprecate Falcon and Starcoder #530

Merged

Conversation

mspronesti
Copy link
Contributor

@mspronesti mspronesti commented Sep 5, 2023

Hi @gventuri,
as a result of this discussion #525, this PR aims at deprecating Falcon and Starcoder. Not sure we should update the docs yet.

Summary by CodeRabbit

  • Refactor: Deprecated the Falcon and Starcoder classes in the pandasai/llm module. Users are advised to use the langchain.llms.HuggingFaceHub class instead for future projects.
  • Documentation: Updated the API documentation (docs/API/llms.md) to reflect these deprecations and provide guidance on the recommended alternatives.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 5, 2023

Walkthrough

This pull request introduces deprecation warnings for the Falcon and Starcoder classes in the pandasai/llm module, recommending users to transition to langchain.llms.HuggingFaceHub. The changes are reflected in the codebase and documentation.

Changes

File Path Summary
pandasai/llm/falcon.py
pandasai/llm/starcoder.py
Deprecation warnings added to __init__ methods. Starcoder now inherits from HuggingFaceLLM. Unused attributes removed and new ones added.
docs/API/llms.md Added deprecation notices for Starcoder and Falcon, suggesting use of HuggingFaceHub.

🐇💻

Code evolves like a river's flow,

Old constructs fade, new ones grow.

Farewell to Falcon, Starcoder too,

HuggingFaceHub, we welcome you! 🎉🥳


Tips

Chat with CodeRabbit Bot (@coderabbitai)

  • Mention @coderabbitai in any review comment for bot assistance. The bot stays silent unless called upon.
  • Note: Review comments are made on code diffs or files, not on the PR overview.

Pause Incremental Reviews

  • Insert @coderabbitai: ignore in the PR description to halt the bot's ongoing reviews. Remove the line to resume.

@mspronesti mspronesti force-pushed the feat/deprecate-falcon-starcoder branch from 80141f1 to bfb039e Compare September 5, 2023 21:38
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 0

Commits Files that changed from the base of the PR and between 5f1b570 and 80141f1 commits.
Files selected for processing (2)
  • pandasai/llm/falcon.py (1 hunks)
  • pandasai/llm/starcoder.py (2 hunks)
Files skipped from review due to trivial changes (1)
  • pandasai/llm/falcon.py
Additional comments (Suppressed): 3
pandasai/llm/starcoder.py (3)
  • 11-14: The import of the warnings module is a new addition to this file. Ensure that it doesn't conflict with any other imports in the same namespace.

  • 31-38: Deprecation warning added for the Starcoder class. This is a significant change and users should be informed about this update in the documentation as well.

  • 40-42: No changes have been made to the type property of the Starcoder class. It continues to return "starcoder" as before.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 1

Commits Files that changed from the base of the PR and between 5f1b570 and bfb039e commits.
Files selected for processing (2)
  • pandasai/llm/falcon.py (2 hunks)
  • pandasai/llm/starcoder.py (2 hunks)
Additional comments (Suppressed): 4
pandasai/llm/falcon.py (3)
  • 8-14: > Note: This review was outside of the patch, but no patch was found that overlapped with it. Original lines [17-14]

The import of the warnings module is a new addition to this file. Ensure that it's used appropriately throughout the codebase.

  • 28-34: Deprecation warning added for the Falcon class. This is a good practice to inform users about deprecated features and suggest alternatives.

  • 35-35: Good use of the super() function to call the parent's initializer while initializing the Falcon class.

pandasai/llm/starcoder.py (1)
  • 11-14: The import of warnings module is new and it's used correctly to issue a deprecation warning in the Starcoder class. The rest of the imports remain unchanged.

Comment on lines 18 to 38

class Starcoder(HuggingFaceLLM):

"""Starcoder LLM API

A base HuggingFaceLLM class is extended to use Starcoder model.

"""
"""Starcoder LLM API (Deprecated: Kept for backwards compatibility)"""

api_token: str
_api_url: str = "https://api-inference.huggingface.co/models/bigcode/starcoder"
_max_retries: int = 30

def __init__(self, **kwargs):
warnings.warn(
"""Starcoder is deprecated and will be removed in a future release.
Please use langchain.llms.HuggingFaceHub instead, although please be
aware that it may perform poorly.
"""
)
super().__init__(**kwargs)

@property
def type(self) -> str:
return "starcoder"
Copy link
Contributor

Choose a reason for hiding this comment

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

The Starcoder class has been updated with a deprecation warning in its __init__ method. This is a good practice when planning to remove or replace a feature in future releases. However, the api_token attribute is still present but according to the PR summary, it should have been removed. Please verify this discrepancy.

-     api_token: str

Also, ensure that all instances where Starcoder is instantiated have been updated to handle the deprecation warning appropriately.

@codecov-commenter
Copy link

Codecov Report

Merging #530 (bfb039e) into main (c986050) will increase coverage by 0.07%.
Report is 1 commits behind head on main.
The diff coverage is 100.00%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the GitHub App Integration for your organization. Read more.

@@            Coverage Diff             @@
##             main     #530      +/-   ##
==========================================
+ Coverage   87.07%   87.14%   +0.07%     
==========================================
  Files          47       47              
  Lines        2019     2030      +11     
==========================================
+ Hits         1758     1769      +11     
  Misses        261      261              
Files Changed Coverage Δ
pandasai/llm/falcon.py 100.00% <100.00%> (ø)
pandasai/llm/starcoder.py 100.00% <100.00%> (ø)

... and 2 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@gventuri
Copy link
Collaborator

gventuri commented Sep 5, 2023

@mspronesti thanks a lot for the PR. I'd say let's wait a couple of weeks before updating the docs. For now I've just mentioned they are deprecated!

@gventuri gventuri merged commit 7232d9b into Sinaptik-AI:main Sep 5, 2023
9 checks passed
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 0

Commits Files that changed from the base of the PR and between bfb039e and ea855d6 commits.
Files selected for processing (1)
  • docs/API/llms.md (1 hunks)
Files skipped from review due to trivial changes (1)
  • docs/API/llms.md

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.

3 participants