-
-
Notifications
You must be signed in to change notification settings - Fork 84
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
Implement EntryPoint as a simple object (remove namedtuple) #339
Conversation
3ab8f91
to
2eca591
Compare
2eca591
to
29ec9c3
Compare
importlib_metadata/__init__.py
Outdated
@@ -159,6 +156,9 @@ class EntryPoint( | |||
|
|||
dist: Optional['Distribution'] = None | |||
|
|||
def __init__(self, *, name, value, 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.
I would recommend going for explicit assignment here for the sake of readability and setting self.dist as well
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.
I'd actually like to keep the name/group/value as the essential attributes separate from the dist, which is optional and auxiliary. Agree with explicit assignment. I'm not sure it's possible.
Co-authored-by: Ronny Pfannschmidt <opensource@ronnypfannschmidt.de>
I'm a little reluctant to move forward with this implementation primarily because of concerns about compatibility. This change removes the ability to construct EntryPoint objects from three positional args. I'm debating whether this change can be introduced without supporting that interface, or if the positional args is worth retaining, or if it should be deprecated. I'm aiming for "preferably one obvious way" to initialize an EntryPoint to limit the support surface. I'm even tempted to disallow construction of EntryPoint objects altogether. Maybe the best approach is to implement a simple constructor, retain compatibility, and consider constraining the interface later. Edit: This approach is the one this PR enacts, attempting to retain compatibility. |
Hello, this change broke stevedore which broke bandit for at least myself (but probably others) https://github.com/common-workflow-language/cwltool/pull/1482/checks?check_run_id=3454232416#step:9:50 Reverting to |
Fixes #337 by eschewing the namedtuple superclass. Allows for removal of PyPy workaround and incidental namedtuple interfaces.