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

fix(load): update error message in case of dataset not found locally and missing api keys #1589

Merged
merged 5 commits into from
Feb 5, 2025

Conversation

ArslanSaleem
Copy link
Collaborator

@ArslanSaleem ArslanSaleem commented Feb 4, 2025

Important

Update error message and add print statement in load() for missing local dataset and API key, with new test added.

  • Behavior:
    • Update error message in load() in pandasai/__init__.py when dataset not found locally and no API key is provided.
    • Add print statement in load() to indicate if dataset was loaded locally or fetched from remote server.
  • Tests:
    • Add test_load_missing_not_found_locally_and_no_remote_key in test_pandasai_init.py to verify updated error message.

This description was created by Ellipsis for d6446b6. It will automatically update as commits are pushed.

@ArslanSaleem ArslanSaleem requested a review from gventuri February 4, 2025 16:54
Copy link

codecov bot commented Feb 4, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.56%. Comparing base (18e4f85) to head (d6446b6).
Report is 4 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1589      +/-   ##
==========================================
+ Coverage   89.54%   89.56%   +0.01%     
==========================================
  Files          67       67              
  Lines        2488     2492       +4     
==========================================
+ Hits         2228     2232       +4     
  Misses        260      260              
Flag Coverage Δ
unittests 89.56% <100.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

❌ Changes requested. Reviewed everything up to d6446b6 in 3 minutes and 2 seconds

More details
  • Looked at 97 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 5 drafted comments based on config settings.
1. pandasai/__init__.py:222
  • Draft comment:
    Consider rewording the error message. For example: "The dataset '{dataset_path}' was not found in your local datasets directory, and no API key was provided. Please set a valid API key to fetch the dataset from the remote server." This improves clarity and conciseness.
  • Reason this comment was not posted:
    Marked as duplicate.
2. pandasai/__init__.py:243
  • Draft comment:
    Avoid using print() in library code. Consider using a proper logging mechanism instead.
  • Reason this comment was not posted:
    Marked as duplicate.
3. tests/unit_tests/test_pandasai_init.py:143
  • Draft comment:
    Ensure that tests are updated to reflect the changes in exception messages and types. The new detailed message should be verified in tests consistently.
  • Reason this comment was not posted:
    Marked as duplicate.
4. tests/unit_tests/test_pandasai_init.py:160
  • Draft comment:
    Test 'test_load_missing_api_url' now expects DatasetNotFound. Ensure this test clearly distinguishes the scenario of a missing remote dataset (HTTP 404) versus missing credentials, as these branches raise different exceptions.
  • Reason this comment was not posted:
    Comment did not seem useful.
5. tests/unit_tests/test_pandasai_init.py:220
  • Draft comment:
    Update the expected error message in 'test_load_without_api_credentials' to match the new, more descriptive error message. Also, consider phrasing improvements like using 'permissions' instead of 'permits'.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.

Workflow ID: wflow_IPwce7Jr2L4rtkVa


Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

pandasai/__init__.py Show resolved Hide resolved
pandasai/__init__.py Show resolved Hide resolved
@gventuri gventuri merged commit f3f698e into main Feb 5, 2025
15 checks passed
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