Skip to content

Conversation

@mattseddon
Copy link
Contributor

@mattseddon mattseddon commented Oct 4, 2022

2/3 main <- #2520 <- this <- #2535

Accommodates the breaking change in treeverse/dvc#8318. We are moving from 2 boolean fields to an enum which will now include:

export enum ExperimentStatus {
  FAILED = 'Failed',
  QUEUED = 'Queued',
  RUNNING = 'Running',
  SUCCESS = 'Success'
}

We cannot merge until the above PR is merged. I will also need to increase the MIN_CLI_VERSION. I will raise follow-up PRs to indicate failed experiments in the table (another behaviour change).

Branch for testing: git+https://github.com/karajan1001/dvc.git@fix7986.

@mattseddon mattseddon self-assigned this Oct 4, 2022
@mattseddon mattseddon changed the base branch from main to move-types-to-contract October 4, 2022 01:44
@mattseddon mattseddon changed the title Use exp show status (drop queued and running - breaking change) Use exp show status (drop queued and running fields - breaking change) Oct 4, 2022
@mattseddon mattseddon changed the title Use exp show status (drop queued and running fields - breaking change) Use status in exp show. Bump min DVC version to 2.x.0 Oct 4, 2022
@mattseddon mattseddon marked this pull request as ready for review October 4, 2022 01:52
@mattseddon mattseddon marked this pull request as draft October 4, 2022 02:10
@mattseddon mattseddon force-pushed the use-exp-show-status branch 2 times, most recently from e1c8039 to 00eff29 Compare October 4, 2022 03:22
@mattseddon mattseddon marked this pull request as ready for review October 4, 2022 03:37
experiment: Experiment
) => {
if (experiment.running) {
if (experiment.status === ExperimentStatus.RUNNING) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of checking for the status everywhere, should we add getters like isRunning(), isQueued()...

@sroy3
Copy link
Contributor

sroy3 commented Oct 4, 2022

Branch for testing: git+https://github.com/karajan1001/dvc.git@fix7986.

For python noobs like me, this means editing the requirements.txt file to be:

git+https://github.com/karajan1001/dvc.git@fix7986
torch==1.12.0
torchvision==0.13.0

@mattseddon mattseddon force-pushed the use-exp-show-status branch from 00eff29 to 3fc2c76 Compare October 4, 2022 18:52
Base automatically changed from move-types-to-contract to main October 4, 2022 19:01
@mattseddon mattseddon force-pushed the use-exp-show-status branch from 3fc2c76 to 55d492c Compare October 4, 2022 19:08
Copy link
Contributor

@julieg18 julieg18 left a comment

Choose a reason for hiding this comment

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

Looks good!

@mattseddon mattseddon changed the title Use status in exp show. Bump min DVC version to 2.x.0 Bump min DVC version to 2.x.0 (Use status from exp show) Oct 6, 2022
@mattseddon mattseddon changed the title Bump min DVC version to 2.x.0 (Use status from exp show) Bump min DVC version to 2.30.0 (Use status from exp show) Oct 6, 2022
@karajan1001
Copy link

@mattseddon
Hello, Matt, release 2.30.0 had already included the JSON out format changes.

https://github.com/iterative/dvc/releases/tag/2.30.0

@mattseddon mattseddon enabled auto-merge (squash) October 11, 2022 01:08
@qlty-cloud-legacy
Copy link

Code Climate has analyzed commit 52ceabf 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.8% (0.0% change).

View more on Code Climate.

@mattseddon mattseddon merged commit 3971336 into main Oct 11, 2022
@mattseddon mattseddon deleted the use-exp-show-status branch October 11, 2022 01:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants