Skip to content

fix: don't route non-widget attribute deletion through remove() - #740

Open
Sanjays2402 wants to merge 1 commit into
pyapp-kit:mainfrom
Sanjays2402:fix/delattr-non-widget-739
Open

fix: don't route non-widget attribute deletion through remove()#740
Sanjays2402 wants to merge 1 commit into
pyapp-kit:mainfrom
Sanjays2402:fix/delattr-non-widget-739

Conversation

@Sanjays2402

Copy link
Copy Markdown

Closes #739

ContainerWidget.__delattr__ unconditionally called self.remove(name), so deleting any plain attribute that is not a current child widget raised ValueError from MutableSequence.remove — including deleting a reference to a widget that had already been removed from the container.

__delattr__ now removes a matching child widget by name and otherwise falls back to object.__delattr__, which restores normal Python semantics (AttributeError for a name that does not exist). The existing test_delete_widget still covers the widget path.

This change was prepared with AI assistance; the regression test was run locally and fails without the fix.

ContainerWidget.__delattr__ unconditionally called self.remove(name),
so deleting any plain attribute that is not a current child widget
raised ValueError from MutableSequence.remove -- including deleting a
reference to a widget that had already been removed from the container.

__delattr__ now looks for a matching child widget by name and removes it,
otherwise falls back to object.__delattr__, which keeps normal Python
semantics (AttributeError for a name that does not exist).
@codecov

codecov Bot commented Jul 31, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.31%. Comparing base (2f08fb3) to head (d3a98f0).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #740      +/-   ##
==========================================
- Coverage   89.32%   89.31%   -0.02%     
==========================================
  Files          40       40              
  Lines        4835     4839       +4     
==========================================
+ Hits         4319     4322       +3     
- Misses        516      517       +1     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

Container del child_widget doesn't check if widget has already been removed.

1 participant