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

Bump min DVC version to 2.55.0 (live metrics for experiments running outside the workspace) #3665

Merged
merged 25 commits into from
Apr 20, 2023

Conversation

mattseddon
Copy link
Member

@mattseddon mattseddon commented Apr 11, 2023

This PR accounts for the upcoming breaking change in the exp show data (iterative/dvc#9170). Please review this PR. I'll be raising follow-ups and merging in (much like #3585).

What this PR currently does:

follow-ups

image

extension/src/experiments/model/collect.ts Outdated Show resolved Hide resolved
extension/src/experiments/model/collect.ts Show resolved Hide resolved
extension/src/experiments/model/index.ts Outdated Show resolved Hide resolved
Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 29837 lines exceeds the maximum allowed for the inline comments feature.

@@ -429,70 +433,6 @@ describe('collectColumns', () => {

expect(onlyHasPrimitiveChild).toBeUndefined()
})

it('should collect all params and metrics from the test fixture', () => {
expect(collectColumns(outputFixture).map(({ path }) => path)).toStrictEqual(
Copy link
Member Author

Choose a reason for hiding this comment

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

[F] This is a duplicate of should return a value equal to the columns fixture when given the output fixture

Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 34855 lines exceeds the maximum allowed for the inline comments feature.

Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 36344 lines exceeds the maximum allowed for the inline comments feature.

@@ -1,12 +1,14 @@
import { commands } from 'vscode'
Copy link
Member Author

Choose a reason for hiding this comment

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

[F] This patch is going away very shortly. I had to resist the urge to just delete it for now.

@@ -58,17 +58,3 @@ export const collectFiles = (

return uniqueValues(acc)
}

export const collectMetricsFiles = (
Copy link
Member Author

Choose a reason for hiding this comment

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

[F] Encapsulated this back in Experiments

Copy link
Member Author

Choose a reason for hiding this comment

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

And move the tests

@@ -1081,9 +1086,9 @@ describe('App', () => {
renderTableWithoutRunningExperiments()

clickRowCheckbox('4fb124a')
clickRowCheckbox('1ba7bcd', true)
Copy link
Member Author

Choose a reason for hiding this comment

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

[F] This label no longer exists, the label is now "workspace" as per experiments running in the workspace.

Copy link
Member Author

Choose a reason for hiding this comment

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

[I] We should wrap these somewhere so they are easier to change if we ever have to do it again.

Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 36360 lines exceeds the maximum allowed for the inline comments feature.

Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 36358 lines exceeds the maximum allowed for the inline comments feature.

} from '../../../cli/dvc/contract'
import {
buildMetricOrParamPath,
FILE_SEPARATOR,
METRIC_PARAM_SEPARATOR
} from '../paths'

export const typedValueTreeEntries = (value: NonNullable<ValueTree>) =>
Copy link
Member Author

Choose a reason for hiding this comment

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

[F] No longer need this now the types have been fixed... or TypeScript can handle typing object entries now.

Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 36122 lines exceeds the maximum allowed for the inline comments feature.

Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 37343 lines exceeds the maximum allowed for the inline comments feature.

Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 37438 lines exceeds the maximum allowed for the inline comments feature.

@mattseddon mattseddon changed the title Accomodate breaking change in exp show Bump min DVC version to 2.5X.0 (live metrics for experiments running outside the workspace) Apr 19, 2023
Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 37438 lines exceeds the maximum allowed for the inline comments feature.

Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 37442 lines exceeds the maximum allowed for the inline comments feature.

@mattseddon mattseddon changed the title Bump min DVC version to 2.5X.0 (live metrics for experiments running outside the workspace) Bump min DVC version to 2.55.0 (live metrics for experiments running outside the workspace) Apr 20, 2023
Copy link

@codeclimate codeclimate bot left a comment

Choose a reason for hiding this comment

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

The PR diff size of 37446 lines exceeds the maximum allowed for the inline comments feature.

@codeclimate
Copy link

codeclimate bot commented Apr 20, 2023

Code Climate has analyzed commit b2c2835 and detected 4 issues on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 2
Duplication 2

The test coverage on the diff in this pull request is 94.4% (85% is the threshold).

This pull request will bring the total coverage in the repository to 94.7% (0.0% change).

View more on Code Climate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
product PR that affects product
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants