live.log_metric: Cast nan and inf to string.

[daavoo]

• Support string metrics (forgot to support them when DVC added support)
• Don't recast to float when sending to Studio.

Closes iterative/studio-support#93

I have manually tested end to end against Studio sending inf/nan from math, numpy, and torch.
The Vega plots in the frontend appear to handle just fine datapoints with those values (the datapoint is skipped).

[dberenbaum]

Code LGTM!

I have manually tested end to end against Studio sending inf/nan from math, numpy, and torch. The Vega plots in the frontend appear to handle just fine datapoints with those values (the datapoint is skipped).

Have you tested VS Code also?

Eventually, this may not be enough, but seems reasonable for now.

[dberenbaum]

Also, how do nan and inf get written before and after this PR?

[daavoo]

Have you tested VS Code also?

Eventually, this may not be enough, but seems reasonable for now.

Yes.
The issue was only when posting Live experiments to Studio because the server was rejecting the request. VSCode / Regular commits were apparently not affected.

Also, how do nan and inf get written before and after this PR?

• Before:

{
    "foo": NaN
}

step    foo
0       nan

• After:

{
    "foo": "nan"
}

step    foo
0       nan

In theory, I could only patch the data sent to Studio.
However, I think always casting to string is safer to ensure consistency between Live experiments and commits and JSON compliance.

[dberenbaum]

Should we add tests for plots in DVC, VS Code, and Studio to ensure we don't break it in the future?

[shcheklein]

VS Code is using JSON5 already and handles it well.

It's not an issue for DVC either since we embed the JS returned by DVC right into HTML and it works fine as expected.

It's an issue with serializing it to send to Studio BE, probably can be an issue on that end with saving it into PG (if that is JSON column), sending it to FE, and on FE (if doesn't use JSON5). So, a long chain of places to check and test (usually with simple fixes though).

[daavoo]

Studio backend is tested and has a cast to string