Collect live plots data from workspace #994
Conversation
|
Code Climate has analyzed commit 405c1d6 and detected 0 issues on this pull request. The test coverage on the diff in this pull request is 100.0% (85% is the threshold). This pull request will bring the total coverage in the repository to 96.4% (0.1% change). View more on Code Climate. |
rogermparent
added
A: plots
| const walkValueTree = ( | ||
| treeOrValue: ValueTree | Value, | ||
| onValue: (value: Value, path: string[]) => void, | ||
| ancestors: string[] = [] | ||
| ): void => { | ||
| if (typeof treeOrValue === 'object') { | ||
| Object.entries(treeOrValue as ValueTree).forEach( | ||
| ([childKey, childValue]) => { | ||
| return walkValueTree(childValue, onValue, [...ancestors, childKey]) | ||
| } | ||
| ) | ||
| } else { | ||
| onValue(treeOrValue as Value, ancestors) | ||
| } | ||
| } |
There was a problem hiding this comment.
This would possibly be better hoisted up as a reusable utility, not sure where though.
|
[Q] Why not have an actual empty state? Just some text that says something along the lines of nothing to display? This looks like an error to me Edit: here is an example of people asking for that behaviour: https://community.plotly.com/t/replacing-an-empty-graph-with-a-message/31497/10 |
There was a problem hiding this comment.
Code change looks fine as a first step. If you are going to merge and close #991 please raise another ticket to revisit/add a real empty state.
|
|
||
| for (const { baseline, ...experimentsObject } of Object.values( | ||
| omit(data, 'workspace') |
There was a problem hiding this comment.
[I] This was the only place that this was used. Please remove the dependency.
|
Also need to update |
I figured we'd want to show the empty plots because we have the list of metrics, giving some worthwhile information even though there are no data points. I think the example is slightly misaligned analog since it's about a single plot in a plotting library while we have multiple plots in a sort of "dashboard" format. Still, there's something to be said for using a message instead. I can put that together quickly if it's preferable and just keep this in mind if we decide otherwise. |
We will also have the situation where the project has no checkpoints. I think we will have
If you think about the dashboard in those terms then 3 will have the true empty state. 1 and 2 probably should just not render the live plots section. |
This PR modifies the collect function for live plots to also collect metrics from the workspace. This is primarily to get a list of metrics even if there are no experiments. The plots webview takes this list and generates empty plots for each metric.
A large part of this PR is to refactor the existing
collectFromMetricsFilefunction, extracting out the recursive walking logic to its own function calledwalkValueTreeand use that to walk the workspace's metrics and add them to the accumulator without recording any other data.Fixes #991, which is part of #978