Skip to content
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.

Sign up for GitHub

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

Jump to bottom

dvc should output prettify json in tty while having --show-json #6712

Closed
eggqq007 opened this issue Sep 30, 2021 · 5 comments · Fixed by #6743
Closed

dvc should output prettify json in tty while having --show-json #6712

eggqq007 opened this issue Sep 30, 2021 · 5 comments · Fixed by #6743
Assignees
@eggqq007
Labels
p2-medium Medium priority, should be done, but less important ui user interface / interaction

Comments

@eggqq007
Copy link
Contributor

Pre-issue: #6646
@eggqq007 eggqq007 mentioned this issue Sep 30, 2021
Merged
2 tasks
@pmrowla pmrowla added p2-medium Medium priority, should be done, but less important ui user interface / interaction labels Oct 1, 2021
@pmrowla
Copy link
Contributor

pmrowla commented Oct 1, 2021

@eggqq007 are you planning on taking this issue yourself (to use the updated write_json in other commands besides list)?

@eggqq007
Copy link
Contributor Author

eggqq007 commented Oct 1, 2021

Yes, I will send the patch out later. :)

@eggqq007 are you planning on taking this issue yourself (to use the updated write_json in other commands besides list)?

@eggqq007
Copy link
Contributor Author

eggqq007 commented Oct 5, 2021 •
edited
Loading

write_json function consumes function from_data in rich. But I found from_data doesn't accept 'default' which just like default in json.dumps. It means it may hit issue while converting class into json.

So I'm considering whether to continue to use from_data to print prettify json.

@skshetry
Copy link
Member

skshetry commented Oct 5, 2021

@eggqq007, maybe we can propose this on rich? Also, for the time being, we could override from_data to support default, see this piece of code: https://github.com/willmcgugan/rich/blob/473eac577f42a243404c3e01ba13ba73c7aaaadd/rich/json.py#L36-L40

@eggqq007
Copy link
Contributor Author

eggqq007 commented Oct 5, 2021

@eggqq007, maybe we can propose this on rich? Also, for the time being, we could override from_data to support default, see this piece of code: https://github.com/willmcgugan/rich/blob/473eac577f42a243404c3e01ba13ba73c7aaaadd/rich/json.py#L36-L40

Thanks for your suggestions. I had created a new discussion at Textualize/rich#1534 last week but get no response.
Yes, re-implement from_data in dvc is a way.

@eggqq007 eggqq007 mentioned this issue Oct 5, 2021
Merged
2 tasks
eggqq007 pushed a commit to eggqq007/dvc that referenced this issue Oct 5, 2021
command: output prettify json in tty
33a6974 
1. update diff.py,experiments.py,metrics.py,params.py,status.py to
print prettify json
2. rewrite write_json in __init__.py to accept default function
3. add new unit test for write_json in tests/unit/ui/test_console.py
4. Fix iterative#6712
skshetry pushed a commit that referenced this issue Oct 5, 2021
@eggqq007
command: output prettify json in tty (#6743)
9c300a0 
1. update diff.py,experiments.py,metrics.py,params.py,status.py to
print prettify json
2. rewrite write_json in __init__.py to accept default function
3. add new unit test for write_json in tests/unit/ui/test_console.py
4. Fix #6712

Co-authored-by: Zhenyu Gao <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@eggqq007 eggqq007

Labels
p2-medium Medium priority, should be done, but less important ui user interface / interaction
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

command: output prettify json in tty eggqq007/dvc
3 participants
@pmrowla @skshetry @eggqq007