Skip to content

bpo-20774: Add a JSON serializer to collections.deque #830

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

Closed
wants to merge 14 commits into from

Conversation

lisroach
Copy link
Contributor

@lisroach lisroach commented Mar 27, 2017

PR for issue 20774, adding a JSON serializer to collections.deque.

https://bugs.python.org/issue20774

@mention-bot
Copy link

@lisroach, thanks for your PR! By analyzing the history of the files in this pull request, we identified @tiran, @benjaminp and @loewis to be potential reviewers.

@Mariatta Mariatta changed the title Issue20774 bpo-20774: Add a JSON serializer to collections.deque Mar 27, 2017
Copy link
Contributor

@rhettinger rhettinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Over this looks good. Can you please add some tests.

@@ -1493,6 +1493,8 @@ encoder_listencode_obj(PyEncoderObject *s, _PyAccu *acc,
/* Encode Python object obj to a JSON term */
PyObject *newobj;
int rv;
PyObject *mo = PyImport_ImportModule("collections");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't you need to decrease the reference counter to mo and deque_type after using them?

@brettcannon brettcannon added the type-feature A feature request or enhancement label Mar 28, 2017
@brettcannon
Copy link
Member

To try and help move older pull requests forward, we are going through and backfilling 'awaiting' labels on pull requests that are lacking the label. Based on the current reviews, the best we can tell in an automated fashion is that a core developer requested changes to be made to this pull request.

If/when the requested changes have been made, please leave a comment that says, I have made the requested changes; please review again. That will trigger a bot to flag this pull request as ready for a follow-up review.

@sakurai-youhei
Copy link

sakurai-youhei commented Jan 14, 2019

For those who arrived here like me when searching quick solution to encode dqeue object into JSON; Below code (as per documentation) shall be good enough.

from collections import deque
from json import dumps, JSONEncoder

class JSONEncoderSupportingArbitraryIterators(JSONEncoder):
    def default(self, o):
        try:
            iterable = iter(o)
        except TypeError:
            pass
        else:
            return list(iterable)
        # Let the base class default method raise the TypeError
        return JSONEncoder.default(self, o)

dumps(deque(range(10)), cls=JSONEncoderSupportingArbitraryIterators)

@carlbordum
Copy link
Contributor

@lisroach are you interested in writting the tests, or may I? :)

@lisroach
Copy link
Contributor Author

lisroach commented Mar 6, 2019

Sorry, just realized I had this sitting for so long! @carlbordum feel free to add more tests if you think of them :)

@@ -3,6 +3,7 @@

from test.support import bigmemtest, _1G


Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe you could remove this blank line

@@ -47,7 +48,6 @@ def __lt__(self, o):
d[1337] = "true.dat"
self.assertEqual(self.dumps(d, sort_keys=True), '{"1337": "true.dat"}')

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this line ?

@csabella
Copy link
Contributor

csabella commented Jun 5, 2019

@lisroach, should this pull request be closed based on the performance benchmarks mentioned on the bug tracker or were you still working on this change? Thanks!

@github-actions
Copy link

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions bot added the stale Stale PR or inactive for long period of time. label Feb 19, 2022
@erlend-aasland
Copy link
Contributor

@lisroach, are you going to follow up this PR?

@erlend-aasland erlend-aasland added the pending The issue will be closed if no feedback is provided label Jun 29, 2022
@github-actions github-actions bot removed the stale Stale PR or inactive for long period of time. label Jun 30, 2022
@kumaraditya303
Copy link
Contributor

Closing because of lack of response and conflicts, feel free to reopen if you want to work on this.

@AA-Turner AA-Turner removed the pending The issue will be closed if no feedback is provided label Apr 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting changes type-feature A feature request or enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.