bpo-29863: Add json.COMPACT constant #72

andrewnester · 2017-02-13T15:50:50Z

Fix for https://bugs.python.org/issue29540

I decided to go with Alternative version provided R.David Murray just to keep things simple

the-knights-who-say-ni · 2017-02-13T15:50:52Z

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA).

Unfortunately our records indicate you have not signed the CLA. For legal reasons we need you to sign this before we can look at your contribution. Please follow these steps to rectify the issue:

Sign the PSF contributor agreement
Wait at least a day and then check "Your Details" on bugs.python.org to see if your account has been marked as having signed the CLA (the delay is due to a person having to manually check your signed CLA)
Reply here saying you have completed the above steps

Thanks again to your contribution and we look forward to looking at it!

berkerpeksag

Thanks for the PR. The new json.COMPACT constant needs to be documented in Doc/library/json.rst (and please add a .. versionadded:: 3.7 marker)

andrewnester · 2017-02-13T19:02:43Z

@berkerpeksag thanks! just added documentation for json.COMPACT

brettcannon · 2017-02-13T20:13:43Z

Doc/library/json.rst

@@ -175,6 +175,8 @@ Basic Usage
   .. versionchanged:: 3.4
      Use ``(',', ': ')`` as default if *indent* is not ``None``.

+   .. versionadded:: 3.7 Instead of ``(',', ':')`` constant ``json.COMPACT`` could be used.


"could be used" -> "can be used"

brettcannon · 2017-02-13T20:15:16Z

Doc/library/json.rst

+
+.. data:: compact
+
+   Constant that could be used as *separators* argument value


A constant that can be used as the *separators* argument to emit a compact serialization.

+1 to Brett's suggestion.

berkerpeksag

Please update the "Compact encoding" example at the top of the page.

berkerpeksag · 2017-02-13T20:37:43Z

Doc/library/json.rst

@@ -175,6 +175,8 @@ Basic Usage
   .. versionchanged:: 3.4
      Use ``(',', ': ')`` as default if *indent* is not ``None``.

+   .. versionadded:: 3.7 Instead of ``(',', ':')`` constant ``json.COMPACT`` could be used.


Also, following the style used in Doc/library/json.rst would be better:

.. versionadded:: 3.7 Instead of ``(',', ':')`` constant ``json.COMPACT`` could be used.

versionchanged would be more appropriate here.

``json.COMPACT`` -> :data:`json.COMPACT`

berkerpeksag · 2017-02-13T20:40:19Z

Doc/library/json.rst

+
+.. data:: compact
+
+   Constant that could be used as *separators* argument value


+1 to Brett's suggestion.

andrewnester · 2017-02-14T07:37:39Z

@berkerpeksag @brettcannon thanks! I've just updated PR according to changes you requested.

berkerpeksag

Please also update JSONEncoder documentation at https://docs.python.org/3/library/json.html#json.JSONEncoder and add a test that passes separators=json.COMPACT to JSONEncoder. Sorry, I missed this in my earlier review.

berkerpeksag · 2017-02-14T13:01:52Z

Doc/library/json.rst

+Constants
+^^^^^^^^^
+
+.. data:: compact


compact -> COMPACT

berkerpeksag · 2017-02-14T13:02:50Z

Doc/library/json.rst

+
+.. data:: compact
+
+   A constant that can be used as the *separators* argument to emit a compact serialization.


Please wrap lines at 80 chars.

andrewnester · 2017-02-14T14:03:57Z

thanks @berkerpeksag ! I've just added changes corresponding to JSONEncoder

codecov · 2017-02-14T14:33:23Z

Codecov Report

Merging #72 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master      #72      +/-   ##
==========================================
+ Coverage   82.38%   82.38%   +<.01%     
==========================================
  Files        1428     1428              
  Lines      351138   351147       +9     
==========================================
+ Hits       289282   289294      +12     
+ Misses      61856    61853       -3

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d50f188...71c48e2. Read the comment docs.

brettcannon · 2017-02-15T18:03:02Z

Doc/library/json.rst

@@ -453,6 +467,9 @@ Encoders and Decoders
   .. versionchanged:: 3.4
      Use ``(',', ': ')`` as default if *indent* is not ``None``.

+   .. versionchanged:: 3.7
+      Instead of ``(',', ':')`` constant :data:`json.COMPACT` can be used.


I don't think a versionchanged field is necessary as nothing about the function actually changed. I think it would be better to update the paragraph above from "To get the most compact JSON representation,
you should specify (',', ':') to eliminate whitespace." to "To get the most compact JSON representation,
you should specify :attr:json.COMPACT to eliminate whitespace.

@brettcannon sounds reasonable! I've just updated it, thanks!

andrewnester · 2017-02-22T13:40:17Z

@berkerpeksag @brettcannon any conclusion on this? :)

brettcannon · 2017-02-24T23:15:58Z

@andrewnester I've been waiting for the discussion of https://bugs.python.org/issue29540 to settle down and be resolved before commenting further.

andrewnester · 2017-03-12T08:58:22Z

@brettcannon looks like discussion stopped here https://bugs.python.org/issue29540 ..
any conclusion on this PR?

brettcannon · 2017-03-13T17:44:04Z

@andrewnester I just pinged the issue to explicitly see what people think about your attribute proposal.

brettcannon · 2017-03-20T22:34:57Z

I opened bpo-29863 to finalize the discussion of the constant since I think it got lost on the older issue that proposed a new compact argument to json.dump().

andrewnester · 2017-04-02T17:07:52Z

@brettcannon thanks! is there any link to discussion or some mailing list?

orsenthil · 2017-04-02T17:18:12Z

@andrewnester : http://bugs.python.org/issue29863

brettcannon · 2017-04-03T20:57:49Z

@andrewnester I just wanted to say sorry for the delay on making a decision for this. As I'm sure you have noticed on the issue it's basically split down the middle as to whether this PR should be accepted or not and so no one has made the final call.

brettcannon · 2017-05-13T19:49:37Z

Closing this since I just closed the issue.

…sole Refactor termios stuff in unix console

…n#72) Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>

the-knights-who-say-ni added the CLA not signed label Feb 13, 2017

berkerpeksag requested changes Feb 13, 2017

View reviewed changes

berkerpeksag changed the title ~~bpo-29540 - Added json.COMPACT constant~~ bpo-29540: Add json.COMPACT constant Feb 13, 2017

brettcannon requested changes Feb 13, 2017

View reviewed changes

berkerpeksag reviewed Feb 13, 2017

View reviewed changes

berkerpeksag reviewed Feb 14, 2017

View reviewed changes

zware removed the CLA not signed label Feb 14, 2017

the-knights-who-say-ni added the CLA signed label Feb 14, 2017

brettcannon requested changes Feb 15, 2017

View reviewed changes

andrewnester added 3 commits February 16, 2017 16:20

29540 - Added json.COMPACT constant

cc0a1f1

Updated documentation for json.COMPACT

32c4b13

Added test to ensure JSONEncoder works with json.COMPACT.

4315176

brettcannon changed the title ~~bpo-29540: Add json.COMPACT constant~~ bpo-29863: Add json.COMPACT constant Mar 20, 2017

brettcannon closed this May 13, 2017

Askaholic mentioned this pull request Jul 22, 2019

Server bothers with whitespace in json messages FAForever/server#166

Closed

jaraco pushed a commit that referenced this pull request Dec 2, 2022

Update aiohttp from 3.0.6 to 3.0.7 (#72)

61b0861

jaraco added a commit to jaraco/cpython that referenced this pull request Feb 17, 2023

Ignore the type. Fixes emergent qa failure and closes python#72.

be21e5a

oraluben pushed a commit to oraluben/cpython that referenced this pull request Jul 14, 2023

feat: executed jitted code for all BB_BRANCH and BB_JUMP (python#72)

81e84a4

gvanrossum mentioned this pull request Aug 22, 2023

heap-use-after-free in _PyFunction_LookupByVersion #108253

Closed

lysnikolaou referenced this pull request in lysnikolaou/cpython Apr 30, 2024

Merge pull request #72 from lysnikolaou/pyrepl-refactor-term-unix-con…

aa9eaf3

…sole Refactor termios stuff in unix console

ngoldbaum mentioned this pull request Sep 23, 2024

Crash running PyO3 tests with --test-threads=1000 #124375

Closed

This was referenced Feb 11, 2025

segmentfault when pip installing setuptools #129992

Open

segmentfault when pip installing setuptools #129993

Closed

AA-Turner added a commit to AA-Turner/cpython that referenced this pull request Apr 22, 2025

t-strings: Reduce undefined behaviour and general improvements (pytho…

5782897

…n#72) Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bpo-29863: Add json.COMPACT constant #72

bpo-29863: Add json.COMPACT constant #72

andrewnester commented Feb 13, 2017

the-knights-who-say-ni commented Feb 13, 2017

berkerpeksag left a comment

andrewnester commented Feb 13, 2017

brettcannon Feb 13, 2017

brettcannon Feb 13, 2017

berkerpeksag Feb 13, 2017

berkerpeksag left a comment

berkerpeksag Feb 13, 2017

berkerpeksag Feb 13, 2017

berkerpeksag Feb 13, 2017

berkerpeksag Feb 13, 2017

andrewnester commented Feb 14, 2017

berkerpeksag left a comment

berkerpeksag Feb 14, 2017

berkerpeksag Feb 14, 2017

andrewnester commented Feb 14, 2017

codecov bot commented Feb 14, 2017

brettcannon Feb 15, 2017

andrewnester Feb 16, 2017

andrewnester commented Feb 22, 2017

brettcannon commented Feb 24, 2017

andrewnester commented Mar 12, 2017

brettcannon commented Mar 13, 2017

brettcannon commented Mar 20, 2017

andrewnester commented Apr 2, 2017

orsenthil commented Apr 2, 2017

brettcannon commented Apr 3, 2017

brettcannon commented May 13, 2017


		.. data:: compact

		Constant that could be used as separators argument value


		.. data:: compact

		A constant that can be used as the separators argument to emit a compact serialization.

bpo-29863: Add json.COMPACT constant #72

bpo-29863: Add json.COMPACT constant #72

Conversation

andrewnester commented Feb 13, 2017

the-knights-who-say-ni commented Feb 13, 2017

berkerpeksag left a comment

Choose a reason for hiding this comment

andrewnester commented Feb 13, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

berkerpeksag left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrewnester commented Feb 14, 2017

berkerpeksag left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrewnester commented Feb 14, 2017

codecov bot commented Feb 14, 2017

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrewnester commented Feb 22, 2017

brettcannon commented Feb 24, 2017

andrewnester commented Mar 12, 2017

brettcannon commented Mar 13, 2017

brettcannon commented Mar 20, 2017

andrewnester commented Apr 2, 2017

orsenthil commented Apr 2, 2017

brettcannon commented Apr 3, 2017

brettcannon commented May 13, 2017