Skip to content

Commit

Permalink
Merge branch 'main' into fix/move-GraphEdf-to-tslib
Browse files Browse the repository at this point in the history
  • Loading branch information
porink0424 authored Jun 12, 2024
2 parents bb4e1a8 + aa182fb commit 99ad058
Show file tree
Hide file tree
Showing 22 changed files with 799 additions and 492 deletions.
16 changes: 14 additions & 2 deletions .github/workflows/typescript-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ jobs:
- run: npm run lint

type-check:
name: Type checking on optuna-dashboard
name: Type checking on optuna-dashboard & standalone_app
runs-on: ubuntu-latest

steps:
Expand All @@ -44,12 +44,24 @@ jobs:
- name: Setup tslib
run: make tslib

- name: Type Check
- name: Type Check optuna_dashboard
working-directory: optuna_dashboard
run: |
npm install
npm run type-check
- name: Build rustlib for standalone_app
working-directory: rustlib
run: |
curl https://rustwasm.github.io/wasm-pack/installer/init.sh -sSf | sh
wasm-pack build --target web
- name: Type Check standalone_app
working-directory: standalone_app
run: |
npm install
npm run type-check
check-package-lock-json:
name: Check package-lock.json
runs-on: ubuntu-latest
Expand Down
10 changes: 8 additions & 2 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,13 @@ Note that `OPTUNA_DASHBOARD_DEBUG=1` makes the server will automatically restart

### Running tests, lint checks and formatters

#### Running Python unit tests
#### Running unit tests for `tslib/`

```
$ make tslib-test
```

#### Running unit tests for `python_tests/`

```
$ pytest python_tests/
Expand Down Expand Up @@ -147,7 +153,7 @@ Please install [wasm-pack](https://rustwasm.github.io/wasm-pack/installer/) and
$ make serve-browser-app
```

Open http://127.0.0.1:9000/
Open http://localhost:5173/


## VS Code Extension
Expand Down
5 changes: 5 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@ tslib:
cd tslib/storage && npm i && npm run build
cd tslib/react && npm i && npm run build

.PHONY: tslib-test
tslib-test: tslib
cd tslib/react/test && python generate_assets.py && npm run test
cd tslib/storage/test && python generate_assets.py && npm run test

.PHONY: serve-browser-app
serve-browser-app: tslib $(RUSTLIB_OUT)
cd standalone_app && npm run watch
Expand Down
5 changes: 4 additions & 1 deletion optuna_dashboard/_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,10 @@ def rename_study(study_id: int) -> dict[str, Any]:
@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:
Expand Down
18 changes: 16 additions & 2 deletions optuna_dashboard/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions optuna_dashboard/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,12 @@
"@tanstack/react-query": "^5.18.1",
"@tanstack/react-table": "^8.16.0",
"@tanstack/react-virtual": "^3.1.2",
"@types/papaparse": "^5.3.14",
"@types/three": "^0.160.0",
"axios": "^1.6.7",
"elkjs": "^0.9.1",
"notistack": "^3.0.1",
"papaparse": "^5.4.1",
"plotly.js-dist-min": "^2.28.0",
"react": "file:../tslib/react/node_modules/react",
"react-dom": "^18.2.0",
Expand Down
4 changes: 2 additions & 2 deletions optuna_dashboard/ts/action.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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})`, {
Expand Down
5 changes: 4 additions & 1 deletion optuna_dashboard/ts/apiClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
17 changes: 13 additions & 4 deletions optuna_dashboard/ts/axiosClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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`, {
Expand Down
13 changes: 12 additions & 1 deletion optuna_dashboard/ts/components/Artifact/DeleteArtifactDialog.tsx
Original file line number Diff line number Diff line change
@@ -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"
Expand Down Expand Up @@ -111,6 +113,7 @@ const DeleteDialog: FC<{
filename,
handleDeleteArtifact,
}) => {
const theme = useTheme()
return (
<Dialog
open={openDeleteArtifactDialog}
Expand All @@ -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">
Expand Down
17 changes: 17 additions & 0 deletions optuna_dashboard/ts/components/Artifact/StudyArtifactCards.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import { StudyDetail } from "ts/types/optuna"
import { actionCreator } from "../../action"
import { ArtifactCardMedia } from "./ArtifactCardMedia"
import { useDeleteStudyArtifactDialog } from "./DeleteArtifactDialog"
import { isTableArtifact, useTableArtifactModal } from "./TableArtifactViewer"
import {
isThreejsArtifact,
useThreejsArtifactModal,
Expand All @@ -35,6 +36,8 @@ export const StudyArtifactCards: FC<{ study: StudyDetail }> = ({ study }) => {
useDeleteStudyArtifactDialog()
const [openThreejsArtifactModal, renderThreejsArtifactModal] =
useThreejsArtifactModal()
const [openTableArtifactModal, renderTableArtifactModal] =
useTableArtifactModal()

const width = "200px"
const height = "150px"
Expand Down Expand Up @@ -96,6 +99,19 @@ export const StudyArtifactCards: FC<{ study: StudyDetail }> = ({ study }) => {
<FullscreenIcon />
</IconButton>
) : null}
{isTableArtifact(artifact) ? (
<IconButton
aria-label="show artifact table"
size="small"
color="inherit"
sx={{ margin: "auto 0" }}
onClick={() => {
openTableArtifactModal(urlPath, artifact)
}}
>
<FullscreenIcon />
</IconButton>
) : null}
<IconButton
aria-label="delete artifact"
size="small"
Expand Down Expand Up @@ -125,6 +141,7 @@ export const StudyArtifactCards: FC<{ study: StudyDetail }> = ({ study }) => {
</Box>
{renderDeleteArtifactDialog()}
{renderThreejsArtifactModal()}
{renderTableArtifactModal()}
</>
)
}
Expand Down
Loading

0 comments on commit 99ad058

Please sign in to comment.