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.

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

Pretty JSON formatter #2863

Merged
merged 4 commits into from
Sep 20, 2020
Merged

Pretty JSON formatter #2863

merged 4 commits into from
Sep 20, 2020

Conversation

liuzhe-lz
Copy link
Contributor

@liuzhe-lz liuzhe-lz commented Sep 4, 2020

This util formats JSON basing on expected width.

Take following object as example:

{
    "batch_size": {"_type": "choice", "_value": [16, 32, 64, 128]},
    "hidden_size": {"_type": "choice", "_value": [128, 256, 512, 1024]},
    "lr": {"_type": "choice", "_value": [0.0001, 0.001, 0.01, 0.1]},
    "momentum": {"_type": "uniform", "_value": [0, 1]}
}

When the width is 60, it will be formatted as:

{
    "batch_size": {
        "_type": "choice",
        "_value": [16, 32, 64, 128]
    },
    "hidden_size": {
        "_type": "choice",
        "_value": [128, 256, 512, 1024]
    },
    "lr": {
        "_type": "choice",
        "_value": [0.0001, 0.001, 0.01, 0.1]
    },
    "momentum": {
        "_type": "uniform",
        "_value": [0, 1]
    }
}

When the width is 40, it will be:

{
    "batch_size": {
        "_type": "choice",
        "_value": [
            16,
            32,
            64,
            128
        ]
    },
    "hidden_size": {
        "_type": "choice",
        "_value": [
            128,
            256,
            512,
            1024
        ]
    },
    "lr": {
        "_type": "choice",
        "_value": [
            0.0001,
            0.001,
            0.01,
            0.1
        ]
    },
    "momentum": {
        "_type": "uniform",
        "_value": [
            0,
            1
        ]
    }
}

The _value arrays will always get collapsed or expanded together, since the util thinks they are "similar".

@QuanluZhang QuanluZhang mentioned this pull request Sep 4, 2020
79 tasks
const fields = nonNull.map(obj => obj[key]).filter(v => v !== undefined);
let elements;
if (detectBatch(fields)) { // look like same field of class instances
elements = batchFormat(fields, indent + ' ', width, key);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we expose the interface of indent width?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As long as it's not configurable in web UI, I think we can make life easier.

@ultmaster
Copy link
Contributor

I thought it is to call JSON.stringify and remove some blanks and newlines. Turned out it was a whole new stringifier.

@QuanluZhang
Copy link
Contributor

@Lijiaoa please help review

@@ -3671,7 +3671,7 @@ debug@^4.0.1, debug@^4.1.0, debug@^4.1.1:
dependencies:
ms "^2.1.1"

debuglog@*, debuglog@^1.0.1:
Copy link
Contributor

Choose a reason for hiding this comment

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

could you remove yarn.lock changes? Because PR #2866 had same changes.

Copy link
Contributor Author

@liuzhe-lz liuzhe-lz Sep 11, 2020

Choose a reason for hiding this comment

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

yarn.lock is automatically modified on build.
I wonder why it's not committed by the PR which changes package.json...

Copy link
Contributor

Choose a reason for hiding this comment

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

emmm, the changes of yarn.lock should be committed by PR which changes package.json. but PR #2866 had done this with other dev update. I think #2866 should be merged into master. so I suggest not upload yarn.lock changes in this PR.

@Lijiaoa
Copy link
Contributor

Lijiaoa commented Sep 10, 2020

PR #2880 had modifiy the rule no-use-before-define

@liuzhe-lz liuzhe-lz closed this Sep 13, 2020
@liuzhe-lz liuzhe-lz reopened this Sep 13, 2020
@liuzhe-lz liuzhe-lz merged commit 8c71813 into microsoft:master Sep 20, 2020
@liuzhe-lz liuzhe-lz deleted the json branch September 20, 2020 09:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants