Skip to content

Conversation

@windelbouwman
Copy link
Contributor

This requests a redraw on the table when updating data from a background task.

Refs #3046

PR Checklist:

  • All new features have been tested
  • All new features have been documented
  • I have read the CONTRIBUTING.md file
  • I will abide by the code of conduct

@HalfWhitt
Copy link
Member

HalfWhitt commented Nov 23, 2025

Thanks for the contribution! This doesn't seem to be working quite right. The test_row_change test in the testbed is crashing on one of the added lines:

index = self._data.index(item)
            ^^^^^^^^^^^^^^^^
AttributeError: 'TableSource' object has no attribute 'index'

And indeed, the TableSource class has no such method (though presumably it could be added)

class TableSource:
def __init__(self, interface):
self.interface = interface
def __len__(self):
return len(self.interface.data)
def __getitem__(self, index):
row = self.interface.data[index]
title, subtitle, icon = (
getattr(row, attr, None) for attr in self.interface.accessors
)
return Row(title=(icon, title), subtitle=subtitle)

I haven't gone poking at this myself, so I can't comment yet on the viability of your approach otherwise, or whether or how it fixes the issue mentioned, but it needs to at least not fail any existing tests.

Speaking of testing, when we fix a bug, we need to make sure we're adding a test that confirms it's fixed. Trying it out by hand can verify it once, but it we also need to make sure it never regresses without us noticing. So for this, we'd need a test that updates the table from a background task, then verifies that the table is in fact displaying the correct thing.

@HalfWhitt

This comment was marked as outdated.

@johnzhou721

This comment was marked as outdated.

@johnzhou721

This comment was marked as outdated.

@HalfWhitt
Copy link
Member

Just noting here that, in order to reduce irrelevant noise, I've hidden discussion related to a confusing but entirely unrelated issue that was fixed by merging in main.

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