Skip to content
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

Remove unique_everseen #347

Closed
wants to merge 1 commit into from
Closed

Conversation

andersk
Copy link

@andersk andersk commented Aug 27, 2021

unique_everseen has some clever optimizations, but only for the key=None case that we don’t use. What do you think about removing it, as a simpler alternative to addressing #342 (review)?

unique_everseen has some clever optimizations, but only for the
key=None case that we don’t use.

Signed-off-by: Anders Kaseorg <[email protected]>
@jaraco
Copy link
Member

jaraco commented Aug 29, 2021

What do you think about removing it...

Thanks again for asking with such respect.

I much prefer the existing implementation for the following reasons:

  • It follows a 'functional' paradigm (within entry_points), deferring the cyclometric complexity to the upstream implementation.
  • It avoids entangling the concern of 'uniqueness' with other concerns about loading entry points.
  • It relies on the tested and reusable abstraction presented in the stdlib.
  • It provides a practical example of the usefulness of unique_everseen (in the stdlib no less).
  • I'm considering exposing unique publicly so that any caller could wrap distributions with it to get unique distributions, and a version that entangles uniqueness with entry points would work against that goal. Even as written, the existing implementation describes a two-liner that would resolve unique distributions from non-unique ones.

If the type annotations have general value, why not contribute them to the Python docs so that anyone using that recipe can benefit from that value? I guess I can anticipate some issues, like maybe the docs authors won't want type annotations in just one of the recipes. I think more_itertools might be less concerned about that, although that project may not wish to deviate from the recipes either. If neither of those projects are responsive to the need for annotations, I'd consider exposing the behavior as jaraco.itertools.unique_everseen and then vendor it here.

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.

2 participants