Add color scale setting #632

gen740 · 2023-09-27T08:05:06Z

Contributor License Agreement

This repository (optuna-dashboard) and Goptuna share common code.
This pull request may therefore be ported to Goptuna.
Make sure that you understand the consequences concerning licenses and check the box below if you accept the term before creating this pull request.

I agree this patch may be ported to Goptuna by other Goptuna contributors.

Reference Issues/PRs

Fixes #426.

What does this implement/fix? Explain your changes.

Add the color theme Setting page to the study detail section.
Add plotly color themes.

codecov · 2023-09-27T08:09:16Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (f838ef5) 69.07% compared to head (280f6a5) 69.39%.
Report is 1 commits behind head on main.

❗ Current head 280f6a5 differs from pull request most recent head 4b74031. Consider uploading reports for the commit 4b74031 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #632      +/-   ##
==========================================
+ Coverage   69.07%   69.39%   +0.31%     
==========================================
  Files          35       35              
  Lines        2393     2369      -24     
==========================================
- Hits         1653     1644       -9     
+ Misses        740      725      -15

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

optuna_dashboard/index.html

optuna_dashboard/ts/components/App.tsx

Co-authored-by: c-bata <[email protected]>

HideakiImamura

Thanks for the PR. I have several comments. PTAL.

optuna_dashboard/ts/components/PlotlyDarkMode.ts

optuna_dashboard/ts/components/Settings.tsx

not522 · 2023-11-01T04:57:24Z

@nabenabe0928 Could you review this PR?

nabenabe0928 · 2023-11-01T05:20:11Z

@nabenabe0928 Could you review this PR?

Sure!

Co-authored-by: Hideaki Imamura <[email protected]>

HideakiImamura

LGTM.

As a follow-up, it would be great to save the configuration of the setting page in the some storage to maintain the configuration over the page reload, etc.

nabenabe0928 · 2023-11-20T06:53:59Z

Sorry for the late response, but now I am working on the review!

nabenabe0928

Thank you for the PR!
I am still working on the review, but the first yet most important point is that GraphRank.tsx is missing the color scale setting (yes, I know that rank plot was added recently), so could you apply the same modification to it as well?

nabenabe0928

Still work in progress, but checked App.tsx and AppDrawer.tsx.

nabenabe0928 · 2023-11-20T07:40:33Z

optuna_dashboard/ts/components/AppDrawer.tsx

+                selected={page === "settings"}
+              >
+                <ListItemIcon sx={styleListItemIcon}>
+                  <SettingsIcon />


Is it better with ColorLens?
Optionally, I would like to discuss if "settings" is an appropriate name for this feature because the name is too general.
If we add any further configurations here in the future, I am definitely for it:)

keisuke-umezawa

I left some review comments. Could you check them?

keisuke-umezawa · 2023-12-31T07:40:50Z

optuna_dashboard/ts/components/AppDrawer.tsx

+            <ListItem key="Settings" disablePadding sx={styleListItem}>
+              <ListItemButton
+                component={Link}
+                to={`${URL_PREFIX}/studies/${studyId}/settings`}


This settings seem to be shared among every study. In that case, the url should be${URL_PREFIX}/settings?

I understand the situation, and you want to remember study_id by url, and go back to previous study page.

How about using pop-up to show setting page? I roughly created my example here, although it is very early implementation and needs improvement.

Some reasons:

You can use ${URL_PREFIX}/settings instead of "${URL_PREFIX}/studies/${studyId}/settings" here.

You can show change during tweaking theme on settings.

Also, the place of setting should be below because it is for setting for all pages, imo.

You can see this example here

optuna_dashboard/ts/components/GraphContour.tsx

keisuke-umezawa · 2024-01-01T02:47:40Z

optuna_dashboard/ts/components/Settings.tsx

+          <MenuItem value={"default"}>Default</MenuItem>
+          <MenuItem value={"seaborn"}>Seaborn</MenuItem>
+          <MenuItem value={"presentation"}>Presentation</MenuItem>
+          <MenuItem value={"ggplot2"}>GGPlot2</MenuItem>


Those templates seem to be for Light mode. Should we only provide templates compatible for dark mode?

Unfortunately, there is only one dark template provided in Plotly's GitHub repository. To offer multiple dark mode templates, we should create original color templates. Therefore, adding (or modifying) an additional dark template could be a suitable follow-up.

Ok. I understand.

keisuke-umezawa · 2024-01-21T06:35:46Z

@gen740
It looks good to me, and work well in my local env.
In the future, it is better to store those settings somewhere even if we can use the settings when reloading.
Could you make an issue for it to keep it in mind after merging it?

@c-bata @nabenabe0928
Do you want to check this PR again as you requested changes?

gen740 · 2024-01-21T17:08:44Z

I've realized there is a bug that needs to be fixed. Could we please wait to merge this until it's resolved?

gen740 · 2024-01-21T17:43:26Z

@keisuke-umezawa
The bug has been fixed. Could you please take another look? Thank you!

Seems resolved

c-bata · 2024-01-22T03:33:47Z

@keisuke-umezawa I've dismissed reviews from both me and @nabenabe0928 since we've been unassigned from this PR. Please merge this PR after checking the latest changes

keisuke-umezawa · 2024-02-06T12:40:22Z

@gen740
Sorry for being late. I set auto-merge. Could you resolve conflicts?

gen740 · 2024-02-06T16:03:18Z

@keisuke-umezawa
I've resolved the conflicts, so please proceed with the merge. Thank you!

…ollow-up Follow up of #632 (Add color scale setting) & #806 (Add "use plotlypy" button)

gen740 added 8 commits September 7, 2023 15:49

Create base UI

48df828

add color palettes

45f5813

Update

30e0ac5

Merge branch 'main' into add_color_scale_setting

f95ac90

Implement color theme

8863946

Change all plotly template.

167f631

Merge branch 'main' into add_color_scale_setting

85e3a1f

Merge branch 'main' into add_color_scale_setting

2006dd0

c-bata self-assigned this Sep 28, 2023

c-bata reviewed Sep 28, 2023

View reviewed changes

optuna_dashboard/index.html Outdated Show resolved Hide resolved

c-bata reviewed Sep 28, 2023

View reviewed changes

optuna_dashboard/ts/components/App.tsx Outdated Show resolved Hide resolved

gen740 and others added 2 commits October 3, 2023 13:33

revert optuna_dashboard/index.html

5e3898b

Delete unnecessary blank line.

45db781

Co-authored-by: c-bata <[email protected]>

c-bata removed their assignment Oct 31, 2023

HideakiImamura self-assigned this Oct 31, 2023

HideakiImamura reviewed Oct 31, 2023

View reviewed changes

optuna_dashboard/ts/components/PlotlyDarkMode.ts Outdated Show resolved Hide resolved

optuna_dashboard/ts/components/Settings.tsx Outdated Show resolved Hide resolved

not522 assigned nabenabe0928 Nov 1, 2023

gen740 and others added 5 commits November 17, 2023 14:31

Fix typo

49fa791

Merge branch 'main' into add_color_scale_setting

70752dd

Update optuna_dashboard/ts/components/Settings.tsx

0d4f0b4

Co-authored-by: Hideaki Imamura <[email protected]>

Format

0346f66

Merge branch 'main' into add_color_scale_setting

5f5efb4

HideakiImamura approved these changes Nov 17, 2023

View reviewed changes

nabenabe0928 previously requested changes Nov 20, 2023

View reviewed changes

nabenabe0928 reviewed Nov 20, 2023

View reviewed changes

HideakiImamura removed their assignment Nov 20, 2023

gen740 added 2 commits December 28, 2023 16:16

Delete unnecessary console.log

5c6baf7

format

cdb8c48

keisuke-umezawa suggested changes Jan 1, 2024

View reviewed changes

gen740 added 5 commits January 11, 2024 14:03

Add coloTheme to useEffect watch list

54706ea

Update setting UI to Popover

b99b877

Use Modal instead of Popover

af6f5cb

Merge branch 'main' into add_color_scale_setting

fc3dda0

format

ab31b63

keisuke-umezawa approved these changes Jan 21, 2024

View reviewed changes

Delete "getColorTemplate" Function

1525e37

c-bata removed their assignment Jan 22, 2024

Merge branch 'main' into add_color_scale_setting

280f6a5

keisuke-umezawa enabled auto-merge February 6, 2024 12:38

keisuke-umezawa disabled auto-merge February 6, 2024 12:39

gen740 added 4 commits February 7, 2024 00:35

[Delete] Delete prettier-ignore

2a7cf2c

Merge branch 'main' into add_color_scale_setting

2c07611

Apply format

9229780

Update biome.json

4b74031

keisuke-umezawa merged commit f32c497 into optuna:main Feb 9, 2024
8 checks passed

gen740 deleted the add_color_scale_setting branch February 9, 2024 01:49

porink0424 mentioned this pull request Feb 14, 2024

Follow up of #632 (Add color scale setting) & #806 (Add "use plotlypy" button) #807

Merged

1 task

c-bata added a commit that referenced this pull request Feb 14, 2024

Merge pull request #807 from porink0424/fix/add-color-scale-setting-f…

afdb1df

…ollow-up Follow up of #632 (Add color scale setting) & #806 (Add "use plotlypy" button)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add color scale setting #632

Add color scale setting #632

gen740 commented Sep 27, 2023

codecov bot commented Sep 27, 2023 •

edited

Loading

HideakiImamura left a comment

not522 commented Nov 1, 2023

nabenabe0928 commented Nov 1, 2023

HideakiImamura left a comment

nabenabe0928 commented Nov 20, 2023

nabenabe0928 left a comment •

edited

Loading

nabenabe0928 left a comment

nabenabe0928 Nov 20, 2023

keisuke-umezawa left a comment

keisuke-umezawa Dec 31, 2023

keisuke-umezawa Dec 31, 2023

keisuke-umezawa Jan 1, 2024

keisuke-umezawa Jan 1, 2024

keisuke-umezawa Jan 1, 2024

gen740 Jan 11, 2024

keisuke-umezawa Jan 21, 2024

keisuke-umezawa commented Jan 21, 2024

gen740 commented Jan 21, 2024

gen740 commented Jan 21, 2024 •

edited

Loading

c-bata commented Jan 22, 2024

keisuke-umezawa commented Feb 6, 2024

gen740 commented Feb 6, 2024

Add color scale setting #632

Add color scale setting #632

Conversation

gen740 commented Sep 27, 2023

Contributor License Agreement

Reference Issues/PRs

What does this implement/fix? Explain your changes.

codecov bot commented Sep 27, 2023 • edited Loading

Codecov Report

HideakiImamura left a comment

Choose a reason for hiding this comment

not522 commented Nov 1, 2023

nabenabe0928 commented Nov 1, 2023

HideakiImamura left a comment

Choose a reason for hiding this comment

nabenabe0928 commented Nov 20, 2023

nabenabe0928 left a comment • edited Loading

Choose a reason for hiding this comment

nabenabe0928 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

keisuke-umezawa left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

keisuke-umezawa commented Jan 21, 2024

gen740 commented Jan 21, 2024

gen740 commented Jan 21, 2024 • edited Loading

c-bata commented Jan 22, 2024

keisuke-umezawa commented Feb 6, 2024

gen740 commented Feb 6, 2024

codecov bot commented Sep 27, 2023 •

edited

Loading

nabenabe0928 left a comment •

edited

Loading

gen740 commented Jan 21, 2024 •

edited

Loading