Skip to content

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

Closed
lisroach wants to merge 14 commits into
python:mainfrom
lisroach:issue20774
Closed

bpo-20774: Add a JSON serializer to collections.deque#830
lisroach wants to merge 14 commits into
python:mainfrom
lisroach:issue20774

Conversation

@lisroach

@lisroachlisroach commented Mar 27, 2017

Copy link
Copy Markdown
Contributor

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

https://bugs.python.org/issue20774

@mention-bot

Copy link
Copy Markdown

@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.

@MariattaMariatta changed the title Issue20774bpo-20774: Add a JSON serializer to collections.dequeMar 27, 2017

@rhettingerrhettinger left a comment

Copy link
Copy Markdown
Contributor

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.

Comment threadModules/_json.c
/* Encode Python object obj to a JSON term */
PyObject *newobj;
int rv;
PyObject *mo = PyImport_ImportModule("collections");

Copy link
Copy Markdown
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?

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

Copy link
Copy Markdown
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

sakurai-youhei commented Jan 14, 2019

Copy link
Copy Markdown

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
Copy Markdown
Contributor

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

@lisroach

Copy link
Copy Markdown
ContributorAuthor

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

Comment threadLib/test/test_json/test_dump.py Outdated

from test.support import bigmemtest, _1G


Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor

@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
Copy Markdown

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

@github-actionsgithub-actionsBot added the stale Stale PR or inactive for long period of time. label Feb 19, 2022
@erlend-aasland

Copy link
Copy Markdown
Contributor

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

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

Copy link
Copy Markdown
Contributor

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

@AA-TurnerAA-Turner removed the pending The issue will be closed if no feedback is provided label Apr 6, 2025
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting changestype-featureA feature request or enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

13 participants

@lisroach@mention-bot@brettcannon@sakurai-youhei@carlbordum@csabella@erlend-aasland@kumaraditya303@matrixise@Birne94@rhettinger@AA-Turner@the-knights-who-say-ni