-
Notifications
You must be signed in to change notification settings - Fork 2.2k
refactor: remove push and pull method from dataframe #1758
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
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1758 +/- ##
==========================================
- Coverage 91.62% 91.20% -0.42%
==========================================
Files 72 72
Lines 2806 2752 -54
==========================================
- Hits 2571 2510 -61
- Misses 235 242 +7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Important
Looks good to me! 👍
Reviewed everything up to daeceb2 in 2 minutes and 30 seconds. Click for details.
- Reviewed
1052lines of code in20files - Skipped
2files when reviewing. - Skipped posting
9draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. README.md:16
- Draft comment:
Removed the push/pull usage examples from the platform section. Ensure a migration note is provided for users. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment suggests ensuring a migration note is provided for users, which is a form of asking the author to ensure something is done. This violates the rule against asking the author to ensure something is done.
2. docs/v3/cli.mdx:27
- Draft comment:
Push and pull command instructions have been removed from the CLI docs. Verify that users are now guided only to use supported commands. - 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.
3. docs/v3/getting-started.mdx:142
- Draft comment:
Removed the push/pull examples and 'Sharing and collaboration' block to reflect the deprecation of these methods. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative, explaining what was done without providing any actionable feedback or suggestions. It doesn't align with the rules for useful comments.
4. docs/v3/share-dataframes.mdx:11
- Draft comment:
Updated the share-dataframes documentation to remove push/pull instructions and emphasize platform-based sharing. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative and does not provide any actionable feedback or suggestions for improvement. It simply states what was done in the update, which violates the rule against making purely informative comments.
5. pandasai/cli/main.py:138
- Draft comment:
Removed the 'push' command from the CLI. Confirm that only the 'pull' command remains for remote dataset operations. - Reason this comment was not posted:
Comment looked like it was already resolved.
6. pandasai/dataframe/base.py:151
- Draft comment:
Deprecated push() and pull() methods have been removed from the DataFrame class. Ensure no external references remain. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is informative and suggests ensuring no external references remain, which violates the rule against asking the PR author to ensure something. It doesn't provide a specific code suggestion or ask for a specific test to be written.
7. tests/integration_tests/local_view/test_local_view.py:138
- Draft comment:
Removed integration tests for push() and pull() functionality. Confirm that test coverage now only addresses the remaining features. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is asking the PR author to confirm that test coverage is sufficient after removing tests, which violates the rule against asking for confirmation or ensuring behavior is intended. It doesn't provide a specific suggestion or point out a specific issue.
8. tests/unit_tests/test_cli.py:194
- Draft comment:
Removed push command tests from CLI unit tests. Verify that all removed functionality is no longer inadvertently referenced. - 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.
9. docs/v3/getting-started.mdx:36
- Draft comment:
Typographical note: Consider changing the phrase "get your free API key signing up at" to "get your free API key by signing up at" for improved clarity and correctness. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_HXX20H4It7hqtx1X
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
Important
Remove
pushandpullmethods fromDataFrameclass and update related documentation and tests.pushandpullmethods fromDataFrameclass inbase.py.pushcommand from CLI inmain.py.pullcommand for dataset retrieval.pushandpullmethods fromREADME.mdanddocs/v3/*.mdxfiles.pushandpullmethods fromtest_dataframe.pyandtest_pull.py.test_cli.pyto reflect removal ofpushcommand.pushandpullinintegration_testsdirectory.This description was created by
for daeceb2. You can customize this summary. It will automatically update as commits are pushed.