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 sort button behavior #127

Merged
merged 1 commit into from
Sep 24, 2023
Merged

Fix sort button behavior #127

merged 1 commit into from
Sep 24, 2023

Conversation

hanjinliu
Copy link
Owner

@hanjinliu hanjinliu commented Sep 24, 2023

  • df.iloc[:, x] was not allowed during sorting. This PR allows this.

Summary by CodeRabbit

  • Bug Fix: Updated error message in _sort function to correctly refer to a "sort" button instead of a "filter" button in tabulous/_qt/_proxy_button.py.
  • Refactor: Enhanced _reset function by adding disconnection of the resetSignal for better control flow in tabulous/_qt/_proxy_button.py.
  • New Feature: Added functionality to check if the table proxy is ordered and if cell references can be created from the table selection in tabulous/_qt/_table/_base/_line_edit.py.
  • Refactor: Implemented __repr__ method in tabulous/_sort_filter_proxy.py for improved debugging and object information display.

@coderabbitai
Copy link

coderabbitai bot commented Sep 24, 2023

Walkthrough

This pull request includes updates to error messaging, signal handling, and condition checking in the tabulous package. It also introduces a new __repr__ method for better debugging and object representation.

Changes

File Summary
.../_proxy_button.py Updated error message in _sort function and improved signal handling in _reset function.
.../_base/_line_edit.py Added import of tabulous module and a new function call (_sl.len_1). Modified an if statement to check the length of column indices.
.../_sort_filter_proxy.py Added a __repr__ method for better object representation and debugging.

🐇💻

In the land of code, where the tabulous dwell,

Changes are made, and they're made quite well.

Messages clearer, signals disconnect,

Conditions checked with utmost respect.

A repr is born, for us to see,

The state of objects, as clear as can be.

So here's to the coders, their efforts we cheer,

For making our software, that much more clear! 🎉🥕


Tips

Chat with CodeRabbit Bot (@coderabbitai)

  • Mention @coderabbitai in any review comment for bot assistance.
  • 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.

Copy link

@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 97572b9 and 6ba69b1.
Files selected for processing (3)
  • tabulous/_qt/_proxy_button.py (2 hunks)
  • tabulous/_qt/_table/_base/_line_edit.py (2 hunks)
  • tabulous/_sort_filter_proxy.py (1 hunks)
Files skipped from review due to trivial changes (1)
  • tabulous/_sort_filter_proxy.py
Additional comments (Suppressed): 4
tabulous/_qt/_table/_base/_line_edit.py (2)
  • 19-22: The new hunk imports a module _slice_op as _sl from tabulous. Ensure that this module is available in the tabulous package and it contains the function len_1 used later in the code.

  • 541-548: The condition for attaching the tooltip has been modified. Previously, the tooltip was attached if the table proxy was not ordered and the size of rsl was not one. Now, the tooltip will be attached if the table proxy is not ordered, or the length of rsl is one, or there are selected columns. This change in logic might affect the behavior of the application. Please verify if this is the intended behavior.

tabulous/_qt/_proxy_button.py (2)
  • 117-125: The line table._set_proxy(f.decompose(index)) has been commented out. This could potentially change the functionality of the _reset method. Please verify if this is intended and ensure that it does not introduce any unexpected behavior.

  • 137-143: A new line self.resetSignal.disconnect() has been added to disconnect the reset signal when the sort button is uninstalled. This seems like a good practice to clean up signals when they are no longer needed, but please verify that there are no side effects from disconnecting this signal at this point in the code.

@hanjinliu hanjinliu merged commit d76ca97 into main Sep 24, 2023
@hanjinliu hanjinliu deleted the fix-sort branch September 24, 2023 07:11
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.

1 participant