Merge pull request #885 from porink0424/feat/checkbox-to-ask-delete-artifacts

Add a checkbox to ask whether to delete artifacts associated to study/trial
This commit is contained in:
c-bata
2024-06-05 16:02:27 +09:00
committed by GitHub
7 changed files with 140 additions and 12 deletions
+4 -1
View File
@@ -180,7 +180,10 @@ def create_app(
@app.delete("/api/studies/<study_id:int>")
@json_api_view
def delete_study(study_id: int) -> dict[str, Any]:
if artifact_store is not None:
data = request.json or {}
remove_associated_artifacts = data.get("remove_associated_artifacts", True)
if artifact_store is not None and remove_associated_artifacts:
delete_all_artifacts(artifact_store, storage, study_id)
try:
+2 -2
View File
@@ -284,9 +284,9 @@ export const actionCreator = () => {
})
}
const deleteStudy = (studyId: number) => {
const deleteStudy = (studyId: number, removeAssociatedArtifacts: boolean) => {
apiClient
.deleteStudy(studyId)
.deleteStudy(studyId, removeAssociatedArtifacts)
.then(() => {
setStudySummaries(studySummaries.filter((s) => s.study_id !== studyId))
enqueueSnackbar(`Success to delete a study (id=${studyId})`, {
+4 -1
View File
@@ -185,7 +185,10 @@ export abstract class APIClient {
studyName: string,
directions: Optuna.StudyDirection[]
): Promise<StudySummary>
abstract deleteStudy(studyId: number): Promise<void>
abstract deleteStudy(
studyId: number,
removeAssociatedArtifacts: boolean
): Promise<void>
abstract renameStudy(
studyId: number,
studyName: string
+13 -4
View File
@@ -116,10 +116,19 @@ export class AxiosClient extends APIClient {
: undefined,
}
})
deleteStudy = (studyId: number): Promise<void> =>
this.axiosInstance.delete(`/api/studies/${studyId}`).then(() => {
return
})
deleteStudy = (
studyId: number,
removeAssociatedArtifacts: boolean
): Promise<void> =>
this.axiosInstance
.delete(`/api/studies/${studyId}`, {
data: {
remove_associated_artifacts: removeAssociatedArtifacts,
},
})
.then(() => {
return
})
renameStudy = (studyId: number, studyName: string): Promise<StudySummary> =>
this.axiosInstance
.post<RenameStudyResponse>(`/api/studies/${studyId}/rename`, {
@@ -1,10 +1,12 @@
import {
Alert,
Button,
Dialog,
DialogActions,
DialogContent,
DialogContentText,
DialogTitle,
useTheme,
} from "@mui/material"
import React, { ReactNode, useState, FC } from "react"
import { Artifact } from "ts/types/optuna"
@@ -111,6 +113,7 @@ const DeleteDialog: FC<{
filename,
handleDeleteArtifact,
}) => {
const theme = useTheme()
return (
<Dialog
open={openDeleteArtifactDialog}
@@ -123,10 +126,18 @@ const DeleteDialog: FC<{
Delete artifact
</DialogTitle>
<DialogContent>
<DialogContentText>
<DialogContentText
sx={{
marginBottom: theme.spacing(2),
}}
>
Are you sure you want to delete an artifact ("
{filename}")?
</DialogContentText>
<Alert severity="warning">
If this artifact is linked to another study or trial, it will no
longer be accessible from that study or trial as well.
</Alert>
</DialogContent>
<DialogActions>
<Button onClick={handleCloseDeleteArtifactDialog} color="primary">
@@ -1,32 +1,40 @@
import {
Alert,
Button,
Checkbox,
Dialog,
DialogActions,
DialogContent,
DialogContentText,
DialogTitle,
FormControlLabel,
} from "@mui/material"
import React, { ReactNode, useState } from "react"
import { useRecoilValue } from "recoil"
import { actionCreator } from "../action"
import { artifactIsAvailable as artifactIsAvailableState } from "../state"
export const useDeleteStudyDialog = (): [
(studyId: number) => void,
() => ReactNode,
] => {
const action = actionCreator()
const artifactIsAvailable = useRecoilValue(artifactIsAvailableState)
const [openDeleteStudyDialog, setOpenDeleteStudyDialog] = useState(false)
const [deleteStudyID, setDeleteStudyID] = useState(-1)
const [removeAssociatedArtifacts, setRemoveAssociatedArtifacts] =
useState(false)
const handleCloseDeleteStudyDialog = () => {
setOpenDeleteStudyDialog(false)
setDeleteStudyID(-1)
setRemoveAssociatedArtifacts(false)
}
const handleDeleteStudy = () => {
action.deleteStudy(deleteStudyID)
setOpenDeleteStudyDialog(false)
setDeleteStudyID(-1)
action.deleteStudy(deleteStudyID, removeAssociatedArtifacts)
handleCloseDeleteStudyDialog()
}
const openDialog = (studyId: number) => {
@@ -42,12 +50,33 @@ export const useDeleteStudyDialog = (): [
handleCloseDeleteStudyDialog()
}}
aria-labelledby="delete-study-dialog-title"
fullWidth
maxWidth="xs"
>
<DialogTitle id="delete-study-dialog-title">Delete study</DialogTitle>
<DialogContent>
<DialogContentText>
Are you sure you want to delete a study (id={deleteStudyID})?
</DialogContentText>
{artifactIsAvailable && (
<>
<FormControlLabel
label="Remove associated trial/study artifacts."
control={
<Checkbox
checked={removeAssociatedArtifacts}
onChange={() => setRemoveAssociatedArtifacts((cur) => !cur)}
/>
}
/>
{removeAssociatedArtifacts && (
<Alert severity="warning">
If artifacts are linked to another study or trial, they will
no longer be accessible from that study or trial as well.
</Alert>
)}
</>
)}
</DialogContent>
<DialogActions>
<Button onClick={handleCloseDeleteStudyDialog} color="primary">
+73
View File
@@ -2,6 +2,7 @@ from __future__ import annotations
import json
import sys
import tempfile
from unittest import TestCase
import optuna
@@ -523,6 +524,78 @@ class APITestCase(TestCase):
self.assertEqual(status, 204)
self.assertEqual(len(get_all_study_summaries(storage)), 1)
@pytest.mark.skipif(
version.parse(optuna.__version__) < version.parse("3.4.0"),
reason="Needs optuna.artifacts",
)
def test_delete_study_with_removing_artifacts(self) -> None:
from optuna.artifacts import upload_artifact
from optuna.artifacts.exceptions import ArtifactNotFound
storage = optuna.storages.InMemoryStorage()
study = optuna.create_study(storage=storage)
with tempfile.TemporaryDirectory() as tmpdir_name:
artifact_store = optuna.artifacts.FileSystemArtifactStore(base_path=tmpdir_name)
with tempfile.NamedTemporaryFile() as f:
f.write(b"dummy")
f.flush()
artifact_id = upload_artifact(study, f.name, artifact_store)
app = create_app(storage, artifact_store)
with artifact_store.open_reader(artifact_id) as reader:
self.assertEqual(reader.read(), b"dummy")
status, _, _ = send_request(
app,
f"/api/studies/{study._study_id}",
"DELETE",
body=json.dumps({"remove_associated_artifacts": True}),
content_type="application/json",
)
self.assertEqual(status, 204)
with self.assertRaises(ArtifactNotFound):
with artifact_store.open_reader(artifact_id) as reader:
reader.read()
self.assertEqual(len(get_all_study_summaries(storage)), 0)
@pytest.mark.skipif(
version.parse(optuna.__version__) < version.parse("3.4.0"),
reason="Needs optuna.artifacts",
)
def test_delete_study_without_removing_artifacts(self) -> None:
from optuna.artifacts import upload_artifact
storage = optuna.storages.InMemoryStorage()
study = optuna.create_study(storage=storage)
with tempfile.TemporaryDirectory() as tmpdir_name:
artifact_store = optuna.artifacts.FileSystemArtifactStore(base_path=tmpdir_name)
with tempfile.NamedTemporaryFile() as f:
f.write(b"dummy")
f.flush()
artifact_id = upload_artifact(study, f.name, artifact_store)
app = create_app(storage, artifact_store)
with artifact_store.open_reader(artifact_id) as reader:
self.assertEqual(reader.read(), b"dummy")
status, _, _ = send_request(
app,
f"/api/studies/{study._study_id}",
"DELETE",
body=json.dumps({"remove_associated_artifacts": False}),
content_type="application/json",
)
self.assertEqual(status, 204)
with artifact_store.open_reader(artifact_id) as reader:
self.assertEqual(reader.read(), b"dummy")
self.assertEqual(len(get_all_study_summaries(storage)), 0)
def test_delete_study_not_found(self) -> None:
storage = optuna.storages.InMemoryStorage()
app = create_app(storage)