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

Allow unsorting data #121

Open
alexmalins opened this issue Jul 15, 2024 · 2 comments
Open

Allow unsorting data #121

alexmalins opened this issue Jul 15, 2024 · 2 comments

Comments

@alexmalins
Copy link
Contributor

alexmalins commented Jul 15, 2024

Now I have some time to work on sort by column for Harlequin.

One component of sorting by column is the ability to unsort the datatable back to the rows' original ordering prior to sorting. For ex, restoring the datatable back to it's original state when a column's sort setting has clicked through "ascending" and "descending" back to no sort order.

A solution to enable via the backend this could be:

  • Add a hidden index column at position 0 to the DataFrame named "_fastdatatable_index" when the backend is instantiated
  • Never expose this column or its contents outwith of the backend, so textual does not render it
    • Requires modifying backend methods like columns, get_column_at, append_rows to hide or handle the hidden column as appropriate
  • When sort is called, the DataFrame hidden column retains the original order by the index that also gets sorted. sort(None) could then be used to unsort, i.e. restore the original ordering when required.

Advantages:

  • This shouldn't require too many lines of code changes
  • It will restore some of the functionality that didn't port over when fastdatatable forked from Textual's original datatable, as unsorting could be achieved via Textual's separation of the data and the order rows are displayed via row_locations

Disadvantages:

  • It creates a protected column name ("_fastdatatable_index") that users of fastdatatable will be forbidden from having in their own tables

Any thoughts Ted? If you're happy for me to proceed, I'll start committing code for more discussion into a branch & draft PR. 🙌

@tconbeer
Copy link
Owner

Hey @alexmalins, Sorry it took me a minute to get back to you.

Because the backend data is largely immutable, we already maintain a few copies of it. I think a simpler approach here would be to just store a copy of the unsorted data before you sort it the first time. The basic change would be:

Class ArrowBackend:
    ...
    def sort(
        self, by: list[tuple[str, Literal["ascending", "descending"]]] | str
    ) -> None:
        """
        by: str sorts table by the data in the column with that name (asc).
        by: list[tuple] sorts the table by the named column(s) with the directions
            indicated.
        """
        if self._unsorted_data is None:
            self._unsorted_data = self.data
        self.data = self.data.sort_by(by)

    def unsort(self) -> None:
        """Mutate self.data to restore the original ordering, before sorting was applied."""
        if self._unsorted_data is not None:
            self.data = self._unsorted_data

Then there will be some other times when you have to invalidate the self._unsorted_data prop, like when adding/deleting rows/columns etc.

@alexmalins
Copy link
Contributor Author

Thanks Ted - I guess I was being too precious in not wanting to store a copy due to the extra memory overhead. But the flipside is not storing & introducing a hidden index introduces more code complexity.

I just pushed #124 with some commits when I was playing around with this the weekend before last. I'll switch it over to your suggested solution and generally tidy it up, as there's some general small fixes and code improvements entwined with the commits adding the hidden index. 👍

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

No branches or pull requests

2 participants