Skip to content

simplify the implementation of firstrest using peel#130

Open
nsajko wants to merge 2 commits intoJuliaCollections:masterfrom
nsajko:peel
Open

simplify the implementation of firstrest using peel#130
nsajko wants to merge 2 commits intoJuliaCollections:masterfrom
nsajko:peel

Conversation

@nsajko
Copy link
Member

@nsajko nsajko commented Mar 4, 2026

Iterators.peel is supported on all Julia versions supported by IterTools. In fact, it was already present with Julia v0.7.

@codecov
Copy link

codecov bot commented Mar 4, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.86%. Comparing base (08ce9ae) to head (482313a).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #130      +/-   ##
==========================================
- Coverage   90.88%   90.86%   -0.03%     
==========================================
  Files           1        1              
  Lines         395      394       -1     
==========================================
- Hits          359      358       -1     
  Misses         36       36              

☔ View full report in Codecov by Sentry.
📢 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.

@nsajko nsajko marked this pull request as ready for review March 4, 2026 13:33
@nsajko nsajko force-pushed the peel branch 2 times, most recently from b1ae311 to dc25294 Compare March 4, 2026 15:45
@omus
Copy link
Contributor

omus commented Mar 4, 2026

Should firstrest be deprecated in favor of Iterators.peel?

@nsajko
Copy link
Member Author

nsajko commented Mar 4, 2026

The doc string of firstrest points to Iterators.peel anyway. IMO there is no need to deprecate, but definitely delete firstrest at the time of a breaking release.

@nsajko nsajko changed the title simplify firstrest using Iterators.peel simplify the implementation of firstrest using Iterators.peel Mar 4, 2026
@nsajko nsajko changed the title simplify the implementation of firstrest using Iterators.peel simplify the implementation of firstrest using peel Mar 4, 2026
@nsajko nsajko force-pushed the peel branch 2 times, most recently from f44202b to 4d3c1df Compare March 6, 2026 09:05
@nsajko nsajko requested a review from oxinabox March 6, 2026 09:58
@oxinabox
Copy link
Member

oxinabox commented Mar 7, 2026

IMO there is no need to deprecate, but definitely delete firstrest at the time of a breaking release.

Sounds like a good reason to deprecate now

nsajko added 2 commits March 7, 2026 15:20
`Iterators.peel` is supported on all Julia versions supported by
IterTools. In fact, it was already present with Julia v0.7.
@nsajko
Copy link
Member Author

nsajko commented Mar 8, 2026

Another option is to simply define firstrest as:

const firstrest = Iterators.peel

The only difference in behavior would be firstrest(empty_iterator) returning nothing instead of throwing.

@nsajko
Copy link
Member Author

nsajko commented Mar 8, 2026

IMO that would be a feature addition, as opposed to a breaking change.

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