Skip to content

feat: Add batch signal support for contiguous item insertions and removals in EventedList - #422

Open
tlambert03 wants to merge 6 commits into
pyapp-kit:mainfrom
tlambert03:batch-sig
Open

feat: Add batch signal support for contiguous item insertions and removals in EventedList#422
tlambert03 wants to merge 6 commits into
pyapp-kit:mainfrom
tlambert03:batch-sig

Conversation

@tlambert03

Copy link
Copy Markdown
Member

playing with explicit support of batch signals, re #419

@codspeed-hq

codspeed-hq Bot commented Jun 10, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 67 untouched benchmarks


Comparing tlambert03:batch-sig (7e94786) with main (cd90d67)

Open in CodSpeed

@codecov

codecov Bot commented Jun 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.76%. Comparing base (cd90d67) to head (7e94786).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #422   +/-   ##
=======================================
  Coverage   99.75%   99.76%           
=======================================
  Files          23       23           
  Lines        2461     2520   +59     
=======================================
+ Hits         2455     2514   +59     
  Misses          6        6           

☔ 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.

@ndxmrb

ndxmrb commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Good stuff! This makes updates for big amounts of data so much easier. Thank you!

@ndxmrb

ndxmrb commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Is there anything in particular you want to add before you merge, or is it ready to go?
The coverage report says the early return is uncovered, but other than that?

To me, it's been a great addition already and using it improves performance significantly. Would love for this to be merged and released 🙂

From the pending EventedModel PR I know your quality standards are very high and there's many things to evaluate beneath the surface. So no pressure, I'm just curious!

@tlambert03

Copy link
Copy Markdown
Member Author

thanks for the ping @ndxmrb (and sorry again for the delay). I think it's fine. the only thing I'd like opinions on (from you, maybe @Czaki) is whether we like the names of the new signals items_inserting / items_inserted ... or whether we want something like batch_inserting / batch_inserted? anyone have any opinions? Will merge in a couple days if not

@tlambert03
tlambert03 marked this pull request as ready for review July 26, 2026 12:58
@ndxmrb

ndxmrb commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Now that you say it, I like batch_inserting / batch_inserted better. To me, that distinguishes the signal for the item (fired every time) from the one for the batch (fired for every contiguous batch) a bit better.

@tlambert03

Copy link
Copy Markdown
Member Author

yeah i think i do too. will change and merge

@tlambert03 tlambert03 closed this Jul 28, 2026
@tlambert03 tlambert03 reopened this Aug 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants