Replace StudySummary to FrozenStudy in serializing #809
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #809 +/- ##
==========================================
- Coverage 69.53% 69.53% -0.01%
==========================================
Files 35 35
Lines 2370 2360 -10
==========================================
- Hits 1648 1641 -7
+ Misses 722 719 -3 ☔ View full report in Codecov by Sentry. |
| summaries = get_study_summaries(storage) | ||
| serialized = [serialize_study_summary(summary) for summary in summaries] | ||
| studies = get_studies(storage) | ||
| serialized = [serialize_frozen_study(s) for s in studies] | ||
| return { | ||
| "study_summaries": serialized, |
There was a problem hiding this comment.
Should I change the key name of study_summaries and study_summary to something?
There was a problem hiding this comment.
Could you add a todo comment? For now, we cannot introduce the breaking changes in JSON Web APIs since our Jupyter Lab extension uses it.
| } | ||
|
|
||
| if summary.datetime_start is not None: | ||
| serialized["datetime_start"] = summary.datetime_start.isoformat() |
There was a problem hiding this comment.
To add datetime_start, I need to add this attribute in FrozenStudy.
There was a problem hiding this comment.
I would say we should drop datetime_start for performance reasons. Could you remove a following line as well?
| summaries = get_study_summaries(storage) | ||
| serialized = [serialize_study_summary(summary) for summary in summaries] | ||
| studies = get_studies(storage) | ||
| serialized = [serialize_frozen_study(s) for s in studies] | ||
| return { | ||
| "study_summaries": serialized, |
There was a problem hiding this comment.
Could you add a todo comment? For now, we cannot introduce the breaking changes in JSON Web APIs since our Jupyter Lab extension uses it.
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.
Reference Issues/PRs
To make a follow-up PR for #783
What does this implement/fix? Explain your changes.
In optuna v3, to get attribute of a study, FrozenStudy was introduced and it is performance efficient.
As we deprecated optuna v2, we can use it now instead of StudySummary.