-
Notifications
You must be signed in to change notification settings - Fork 301
Optimize all_equal recipe #899
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
Conversation
Did you overlook my benchmark where this was slower than the current version for the case of one group? (Which is the most relevant one for the mentioned usage un |
Thanks for the comment. To be totally transparent, I am going to favor Raymond's suggestions in almost every case – this library is a wrapper around his work, and unless I have a big disagreement with one of his ideas, I'm going to implement them. You have a good track record of contributing to this library as well, and as such I am very open to your suggestion too. That said, I generally discount microbenchmarks on a single machine with un-representative inputs, which is what appears to be the basis for the "this is slower" claim. |
Pochmann later posted a for/for variant that is much better than his other proposals. After changing the variable name from
I don't think we can do better than this. It is short and relies only on the most basic of Python features. It is a little less straight-forward than the |
As shown in the issue, I also reproduced my findings on another machine. And reproduced Raymond's disagreeing findings on the same machine when measuring his command line way without a function. I still don't know why measuring that way gives such very different results, but that way isn't the realistic way. The real About "un-representative inputs": They do represent all cases: No groups, one group, more than one group. And all solutions under consideration use I also like the normal version of my "gross" hack: def all_equal(iterable, key=None):
seen_one = False
for _ in groupby(iterable, key):
if seen_one:
return False
seen_one = True
return True It's similarly fast:
Benchmark scriptdef current(iterable, key=None):
return len(list(islice(groupby(iterable, key), 2))) <= 1
def Raymond(iterable, key=None):
iterator = groupby(iterable, key=key)
try:
next(iterator)
next(iterator)
except StopIteration:
return True
return False
def Stefan_hack(iterable, key=None):
for _ in groupby(iterable, key):
if iterable is None:
return False
iterable = None
return True
def Stefan_loops(iterable, key=None):
iterator = groupby(iterable, key)
for first in iterator:
for second in iterator:
return False
return True
def Stefan_seen(iterable, key=None):
seen_one = False
for _ in groupby(iterable, key):
if seen_one:
return False
seen_one = True
return True
funcs = [current, Raymond, Stefan_hack, Stefan_loops, Stefan_seen]
iterables = (), (1,), (1, 2)
from timeit import timeit
from statistics import mean, stdev
import sys
import random
from itertools import *
for iterable in iterables:
print(f'{iterable = }')
times = {f: [] for f in funcs}
def stats(f):
ts = [t * 1e9 for t in sorted(times[f])[:10]]
return f' {mean(ts):3.0f} ± {stdev(ts):3.1f} ns '
for _ in range(100):
random.shuffle(funcs)
for f in funcs:
t = timeit(lambda: f(iterable), number=1000) / 1000
times[f].append(t)
for f in sorted(funcs, key=stats):
print(f(iterable), stats(f), f.__name__)
print()
print('Python:', sys.version) |
Stefan, take the Bo, aside from the two other hacky variants proposed, I don't think there are any wrong choices here. My order of preference is 1) the Though not currently the fastest, the |
This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [more-itertools](https://redirect.github.com/more-itertools/more-itertools) | `==10.4.0` -> `==10.5.0` | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>more-itertools/more-itertools (more-itertools)</summary> ### [`v10.5.0`](https://redirect.github.com/more-itertools/more-itertools/releases/tag/v10.5.0) [Compare Source](https://redirect.github.com/more-itertools/more-itertools/compare/v10.4.0...v10.5.0) #### What's Changed - Optimize all_equal recipe by [@​bbayles](https://redirect.github.com/bbayles) in [https://github.com/more-itertools/more-itertools/pull/899](https://redirect.github.com/more-itertools/more-itertools/pull/899) - Reduce groupby.**next** calls in all_equal by [@​bbayles](https://redirect.github.com/bbayles) in [https://github.com/more-itertools/more-itertools/pull/903](https://redirect.github.com/more-itertools/more-itertools/pull/903) - Fix types.UnionType by [@​bbayles](https://redirect.github.com/bbayles) in [https://github.com/more-itertools/more-itertools/pull/905](https://redirect.github.com/more-itertools/more-itertools/pull/905) - Version 10.5.0 by [@​bbayles](https://redirect.github.com/bbayles) in [https://github.com/more-itertools/more-itertools/pull/906](https://redirect.github.com/more-itertools/more-itertools/pull/906) **Full Changelog**: more-itertools/more-itertools@v10.4.0...v10.5.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - "every weekend" in timezone Etc/UTC, Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR was generated by [Mend Renovate](https://mend.io/renovate/). View the [repository job log](https://developer.mend.io/github/canonical/charmcraft). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOC41OS4yIiwidXBkYXRlZEluVmVyIjoiMzguNTkuMiIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsiZGVwZW5kZW5jaWVzIl19--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
…#1885) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [more-itertools](https://redirect.github.com/more-itertools/more-itertools) | `==10.4.0` -> `==10.5.0` | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>more-itertools/more-itertools (more-itertools)</summary> ### [`v10.5.0`](https://redirect.github.com/more-itertools/more-itertools/releases/tag/v10.5.0) [Compare Source](https://redirect.github.com/more-itertools/more-itertools/compare/v10.4.0...v10.5.0) #### What's Changed - Optimize all_equal recipe by [@​bbayles](https://redirect.github.com/bbayles) in [https://github.com/more-itertools/more-itertools/pull/899](https://redirect.github.com/more-itertools/more-itertools/pull/899) - Reduce groupby.**next** calls in all_equal by [@​bbayles](https://redirect.github.com/bbayles) in [https://github.com/more-itertools/more-itertools/pull/903](https://redirect.github.com/more-itertools/more-itertools/pull/903) - Fix types.UnionType by [@​bbayles](https://redirect.github.com/bbayles) in [https://github.com/more-itertools/more-itertools/pull/905](https://redirect.github.com/more-itertools/more-itertools/pull/905) - Version 10.5.0 by [@​bbayles](https://redirect.github.com/bbayles) in [https://github.com/more-itertools/more-itertools/pull/906](https://redirect.github.com/more-itertools/more-itertools/pull/906) **Full Changelog**: more-itertools/more-itertools@v10.4.0...v10.5.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - "every weekend" in timezone Etc/UTC, Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR was generated by [Mend Renovate](https://mend.io/renovate/). View the [repository job log](https://developer.mend.io/github/canonical/charmcraft). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOC41OS4yIiwidXBkYXRlZEluVmVyIjoiMzguNTkuMiIsInRhcmdldEJyYW5jaCI6ImhvdGZpeC8zLjIiLCJsYWJlbHMiOlsiZGVwZW5kZW5jaWVzIl19--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [more-itertools](https://redirect.github.com/more-itertools/more-itertools) | `==10.4.0` -> `==10.5.0` | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>more-itertools/more-itertools (more-itertools)</summary> ### [`v10.5.0`](https://redirect.github.com/more-itertools/more-itertools/releases/tag/v10.5.0) [Compare Source](https://redirect.github.com/more-itertools/more-itertools/compare/v10.4.0...v10.5.0) #### What's Changed - Optimize all_equal recipe by [@​bbayles](https://redirect.github.com/bbayles) in [https://github.com/more-itertools/more-itertools/pull/899](https://redirect.github.com/more-itertools/more-itertools/pull/899) - Reduce groupby.**next** calls in all_equal by [@​bbayles](https://redirect.github.com/bbayles) in [https://github.com/more-itertools/more-itertools/pull/903](https://redirect.github.com/more-itertools/more-itertools/pull/903) - Fix types.UnionType by [@​bbayles](https://redirect.github.com/bbayles) in [https://github.com/more-itertools/more-itertools/pull/905](https://redirect.github.com/more-itertools/more-itertools/pull/905) - Version 10.5.0 by [@​bbayles](https://redirect.github.com/bbayles) in [https://github.com/more-itertools/more-itertools/pull/906](https://redirect.github.com/more-itertools/more-itertools/pull/906) **Full Changelog**: more-itertools/more-itertools@v10.4.0...v10.5.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - "every weekend" in timezone Etc/UTC, Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR was generated by [Mend Renovate](https://mend.io/renovate/). View the [repository job log](https://developer.mend.io/github/canonical/charmcraft). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOC41OS4yIiwidXBkYXRlZEluVmVyIjoiMzguNTkuMiIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsiZGVwZW5kZW5jaWVzIl19--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
This PR updates
all_equal
with @rhettinger's new version.Closes #896