-
-
Notifications
You must be signed in to change notification settings - Fork 82
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
Feature/entry points by group and name #278
Conversation
…roup then by name.
Hyrum's law strikes again. Seems Flake8 is relying on the output of |
04b3d28
to
28adeb8
Compare
So this finding begs the question, what other use-cases are there out there that expect |
Here's my answer to those questions - I recommend to merge this change as written, cut a release of |
@sigmavirus24: Would you care to review this change, given that it was inspired by a |
I plan to merge this as early as Friday unless I hear from Barry or Ian that they'd like me to hold off and await their review. |
@asottile you rewrote flake8 to use importlib-metadata. Thoughts? |
importlib_metadata/__init__.py
Outdated
""" | ||
|
||
def __getitem__(self, group) -> EntryPoints: | ||
return EntryPoints(ep for ep in self if ep.group == group) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A bit unfortunate that this is O(N)
on every key access -- this will likely surprise consumers who may access this in a loop
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. Perhaps there's a more efficient way to handle it. If there's a use-case that demands it, I'd be happy to investigate an optimization. I do expect this approach to be less complex than sorting and grouping by group as before.
00f10d9
to
9448e13
Compare
and actually, taking a step back you argued exactly the opposite point when I pointed this out originally (that it can't be removed): #97 (comment) |
I'm not following this concern exactly (I'm unsure what point was argued). I re-read that thread from that comment and it seems that I was not adamant about the approach, but ultimately settled on the current compromise that I'd now like to amend. At the time, I didn't have another proposal that met the goals, but I believe this approach does. Your concerns were relevant then and are now. If you have a concern that's not addressed in another thread, could you re-state it and what you recommend? |
25c9634
to
c9adbca
Compare
c9adbca
to
71fd4a7
Compare
I've been considering an alternative experience that uses parameters to
In this form, the user will always get an iterator of EntryPoint objects and they can filter on group or name but otherwise just get an iterator of EntryPoints. The result would need to be wrapped in some object to provide backward compatibility, but that wrapper would eventually be removed. A materialized list could be used in place of an iterator, although that approach would give the caller less control over the execution. Would that approach be preferable? |
… single collection that can select on 'name' or 'group' or possibly other attributes. Expose that selection in the 'entry_points' function.
With all due respect, I am listening, and I care very much about your opinion, and I want to find a solution and preferably one that doesn't take several years to implement. Thusfar, your recommendations have primarily been "don't change anything," which is why I've focused on explaining the motivations for the change. I've since added more context in separate issues to help distill the motivations and elucidate the challenges. I'm sorry you've disengaged, but I'll try to push forward without you and honor the concerns you've raised.
I believe that's what I've done. The current approach retains the My plan currently is to come up with a design that's largely compatible, has easy adjustments for rare incompatibilities, and declares this change with a backward-incompatible release. |
e411927
to
9d55a33
Compare
…p in compatibility shim.
I've been thinking about the compatibility concerns and I have a new idea - it should be possible to cut a fully compatible release of these changes, exposing the new |
e0ec362
to
a474726
Compare
…rd compatibilty to the new interfaces without sacrificing backward compatibility.
a474726
to
2db4dad
Compare
In this latest version, I've come up with a fully compatible release with forward compatibility for the new, simpler, and more flexible interface. I'd like to get it out to ensure it's stable. Thanks for the comments - they really helped me figure out a better transition here. |
Support multiplexed descendants in MultiplexedPath.
In pypa/twine#728, I learned that the
dict
hack for resolving a group of entry points by name is unintuitive, violates static type checks, and still doesn't provide an elegant way to resolve a set of entry points by group and name.This finding inspired me to create this patch to provide an even nicer experience. See the changes to the docs and tests for examples of the improved usage.
The change is backward compatible for usages under test, but deprecates the
__iter__
on EntryPoint anddict
-based construction.