-
-
Notifications
You must be signed in to change notification settings - Fork 85
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
pypy2: EntryPoint object repr results in infinite recursion #97
Comments
In GitLab by @asottile on Nov 29, 2019, 18:17 mentioned in commit asottile/flake8@6b764d0 |
In GitLab by @asottile on Nov 29, 2019, 18:18 mentioned in merge request pycqa/flake8!389 |
In GitLab by @asottile on Nov 30, 2019, 02:33 I'd be interested in working on this, the root cause is the same as #96 -- generally when extending I'm wondering if at this point it's too late to back out |
In GitLab by @jaraco on Nov 30, 2019, 10:25 In the aforementioned commit, I added a test. The test only fails on PyPy 2, but passes on PyPy 3. |
In GitLab by @jaraco on Nov 30, 2019, 10:26 changed title from pypy: EntryPoint object repr results in infinite recursion to pypy{+2+}: EntryPoint object repr results in infinite recursion |
In GitLab by @jaraco on Nov 30, 2019, 10:31 In the bugfix/pickleable-ep branch, I have a fix for #96 (I'm just waiting on a fix for #100 before submitting). Given the limited impact (doesn't affect late PyPy 3) of this issue, I'm less inclined to re-design |
In GitLab by @jaraco on Nov 30, 2019, 10:38 These are the tested assertions that fail when removing
It feels to me like the value of constructing dicts of EntryPoints is more valuable than the edge cases that it trips up, especially if those edge cases can be addressed directly and cleanly. |
In GitLab by @asottile on Nov 30, 2019, 15:29 Here's an example which is impossible to fix:
|
In GitLab by @asottile on Nov 30, 2019, 15:31 (typed that on phone, here's the actual error):
|
In GitLab by @jaraco on Nov 30, 2019, 17:02 mentioned in commit 741ed9eeca75f288ee5a0bbbb341963aff4c6d9a |
In GitLab by @jaraco on Nov 30, 2019, 17:02 mentioned in commit 3b999e408591f26ee14fe9bacccd511a2784c46c |
In GitLab by @jaraco on Nov 30, 2019, 17:02 mentioned in commit b742833623ad7b9d42e65086d5731a6a39625ba5 |
In GitLab by @jaraco on Nov 30, 2019, 17:30 mentioned in commit 645b25330dbb847da8a02de3b47ed6414daa605d |
In GitLab by @jaraco on Nov 30, 2019, 17:30 mentioned in commit a8b9a314120811b6ee48b53d9889d5900df91679 |
In GitLab by @jaraco on Nov 30, 2019, 17:30 mentioned in commit 727922c65cf86578d10274c239d20dfaf477d23b |
In GitLab by @jaraco on Nov 30, 2019, 17:31 In the feature/entrypoints-not-tuple branch, I've explored this latest manifestation, assembling the various requirements from this an #96 and consolidated those tests. EntryPoints were never intended to be serializable to JSON and the fact that In any case, I would expect if one wants to JSON-serialize an EntryPoint, they should have to write a custom encoder. The fact that a "Circular reference is detected" is as fine an error as any really. The real question is - which is better - having a |
In GitLab by @asottile on Nov 30, 2019, 17:47 json serialization is just one example, there's probably more if I were to look into it :S. as far as I can tell the reimplemeting the immutability is probably the best approach (will probably also want |
In GitLab by @jaraco on Nov 30, 2019, 18:24 It's not documented, but it is an intentional feature that's captured by 5 tests. I wanted it to be trivially easy for a consumer to find entry points by name (and to construct dictionaries of entry points by name). I'm seeking to avoid users having to import another helper function for this purpose (as they must with entrypoints) and the associated namespacing challenges that brings (do you put entry-point utility functions alongside other distribution functions or encapsulate them somewhere). The goal was for users to have one function to retrieve entry points and be able to readily use the result for a variety of use-cases. I'm thinking we should document and advertise and encourage the I don't think it's enough to illustrate (increasingly obscure) uses that break this design. One also needs to present an alternative - how would you recommend satisfying both the list-of-entrypoints and dict-of-entrypoints-by-name use-cases? Personally, I'm really proud of the existing implementation. It allows a dict-of-entrypoints-by-name to be constructed from a list-of-entrypoints by just applying a built-in Of the options we have, my preference would be to keep the namedtuple (so we get hashability, equality, repr, immutability for free) and apply the workarounds for use-cases that specifically need supported (pickleability for flake8 seems quite necessary, repr on pypy2 maybe less so, json support and other issues unlikely to be needed). My second preference would be to re-implement most of namedtuple to avoid those pitfalls, but retain the |
In GitLab by @asottile on Nov 30, 2019, 18:28 yep, I'll take a stab at this -- thanks for the thorough writeup 👍 -- the |
In GitLab by @jaraco on Nov 30, 2019, 18:37 Another thing I just realized - the order of the results from |
In GitLab by @jaraco on Dec 1, 2019, 15:46 I've decided to go ahead and merge the surgical fixes for this and release it as 1.1.0, but that doesn't preclude taking another approach. Depending on what you find in your draft, feel free to re-open this issue or open another issue about the more general problem. |
In GitLab by @jaraco on Dec 1, 2019, 15:53 closed via merge request !100 |
In GitLab by @jaraco on Dec 1, 2019, 16:04 mentioned in commit c3f0769d6320e6f03c5bbcddefa6f36e7f087c2a |
In GitLab by @asottile on Dec 4, 2019, 17:03 yep, sounds good -- I think this approach is fine for now -- I'll do some more testing with other apis and see if there's something more useful than that. I still think the |
In GitLab by @asottile on Nov 29, 2019, 18:08
The text was updated successfully, but these errors were encountered: