bpo-31553: add --json-lines option to json.tool #10051
Conversation
The change LGTM, but I'm not 100% sure about the usefulness of the feature :-) See discussion at https://bugs.python.org/issue31553
Lib/json/tool.py
Outdated
| with infile: | ||
| try: | ||
| obj = json.load(infile) | ||
| if jsonlines: | ||
| objs = [json.loads(line) for line in infile] |
There was a problem hiding this comment.
Instead of loading all lines and only later dump all objects, it may be better to: load on object, dump it, then load the second object, etc.
The behavior difference is that first JSONs are written into outfile if they are valid, but if the last one is invalid. With the current code, it's all-or-nothing. Maybe it's done on purpose?
The other advantage of working on a single object is to reduce the memory footprint when working with large JSON files.
There was a problem hiding this comment.
Good idea.
Just make objs an iterator:
objs = (json.loads(line) for line in infile)or
objs = map(json.loads, infile)There was a problem hiding this comment.
Wow, that's great. Thank you for your guidance.
Lib/json/tool.py
Outdated
| @@ -26,19 +26,26 @@ def main(): | |||
| help='write the output of infile to outfile') | |||
| parser.add_argument('--sort-keys', action='store_true', default=False, | |||
| help='sort the output of dictionaries alphabetically by key') | |||
| parser.add_argument('--jsonlines', action='store_true', default=False, | |||
There was a problem hiding this comment.
I suggest to name the option --json-lines or just --lines.
There was a problem hiding this comment.
--json-lines named similar to the existing --sort-keys,it will be more suitable.
| @@ -26,19 +26,26 @@ def main(): | |||
| help='write the output of infile to outfile') | |||
| parser.add_argument('--sort-keys', action='store_true', default=False, | |||
| help='sort the output of dictionaries alphabetically by key') | |||
| parser.add_argument('--jsonlines', action='store_true', default=False, | |||
| help='parse input using the jsonlines format') | |||
There was a problem hiding this comment.
Maybe 'parse every input line as separate JSON object'?
There was a problem hiding this comment.
Write whatsnew in 3.8?
Lib/json/tool.py
Outdated
| with infile: | ||
| try: | ||
| obj = json.load(infile) | ||
| if jsonlines: | ||
| objs = [json.loads(line) for line in infile] |
There was a problem hiding this comment.
Good idea.
Just make objs an iterator:
objs = (json.loads(line) for line in infile)or
objs = map(json.loads, infile)
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
Doc/whatsnew/3.8.rst
Outdated
| @@ -135,6 +135,11 @@ by right-clicking the button. (Contributed by Tal Einat in :issue:`1529353`.) | |||
| The changes above have been backported to 3.7 maintenance releases. | |||
|
|
|||
|
|
|||
| json.tool | |||
| ------- | |||
There was a problem hiding this comment.
Add an empty line after "---". And the number of minuses should match the length of the header.
There was a problem hiding this comment.
I have limited knowledge about Doc.Thank you for your patience.
Doc/whatsnew/3.8.rst
Outdated
| @@ -135,6 +135,11 @@ by right-clicking the button. (Contributed by Tal Einat in :issue:`1529353`.) | |||
| The changes above have been backported to 3.7 maintenance releases. | |||
|
|
|||
|
|
|||
| json.tool | |||
| ------- | |||
| Add option ``--json-lines`` to parse every input line as separate JSON object. | |||
There was a problem hiding this comment.
Add "(Contributed by ... in :issue:`...`.)"
|
I have made the requested changes; please review again. |
|
Thanks for making the requested changes! @serhiy-storchaka: please review the changes made to this pull request. |
hongweipeng
changed the title
|
@matrixise It's my full name :) |
|
ping |
|
@serhiy-storchaka: Please replace |
|
Damn it! I have pressed Enter by mistake while edited commit message. Thank you for your contribution @hongweipeng! |
|
@serhiy-storchaka Oh!It is a beautiful mistake for me. Will you revoke this merger? |
|
It can't be revoked without making larger harm. |
https://bugs.python.org/issue31553