fix: unified graph captions. #410

emori-ctc · 2023-10-27T03:30:16Z

Description

Notes for reviewers

Pre-review checklist for the PR author

I've confirmed the contribution guidelines.
The PR is ready for review.

In-review checklist for the PR reviewers

The PR reviewers must check the checkboxes below before approval.

The PR has been properly tested.
The PR has been reviewed.

Post-review checklist for the PR author

The PR author must check the checkboxes below before merging.

(Optional) The PR has been properly tested with CARET_report verification.
There are no open discussions or they are tracked via tickets.
The PR is ready for merge.

After all checkboxes are checked, anyone who has write access can merge the PR.

Signed-off-by: emb4 <[email protected]>

rokamu623

As shown below, the hoge case is also displayed when create_latency_timeseries is visualised.

The same applies for create_frequency_timeseries and create_period_timeseries.
Resolve this issue.

We also list two additional trivial mistake as follow:

rokamu623 · 2023-10-30T03:19:20Z

src/caret_analyze/plot/visualize_lib/bokeh/stacked_bar.py

        target_objects = self._metrics.target_objects
        data, y_labels = self._metrics.to_stacked_bar_data()
-        title: str = f"Stacked bar of '{getattr(target_objects, 'path_name')}'"
+        title: str = f'Stacked bar of  Response Time --- {self._case} case ---'


Together with latency, etc., it should be lower-cased.

Suggested change

title: str = f'Stacked bar of Response Time --- {self._case} case ---'

title: str = f'Stacked bar of response time --- {self._case} case ---'

rokamu623 · 2023-10-30T03:22:09Z

src/caret_analyze/plot/visualize_lib/bokeh/timeseries.py

+        title: str = f'Histogram of {data_type}'if self._case is None else\
+            f'histogram of {data_type} --- {self._case} case ---'


Suggested change

title: str = f'Histogram of {data_type}'if self._case is None else\

f'histogram of {data_type} --- {self._case} case ---'

title: str = f'Timeseries of {data_type}'if self._case is None else\

f'Timeseries of {data_type} --- {self._case} case ---'

Signed-off-by: emb4 <[email protected]>

rokamu623 · 2023-11-02T09:06:28Z

src/caret_analyze/plot/visualize_lib/bokeh/stacked_bar.py

        target_objects = self._metrics.target_objects
        data, y_labels = self._metrics.to_stacked_bar_data()
-        title: str = f"Stacked bar of '{getattr(target_objects, 'path_name')}'"
+        title: str = f'Stacked bar of response Time --- {self._case} case ---'


Not fixed.

Suggested change

title: str = f'Stacked bar of response Time --- {self._case} case ---'

title: str = f'Stacked bar of response_time --- {self._case} case ---'

Signed-off-by: emb4 <[email protected]>

rokamu623

Careless mistakes can still be found. Please fix.

rokamu623 · 2023-11-06T04:57:32Z

src/caret_analyze/plot/visualize_lib/bokeh/bokeh.py

@@ -214,7 +214,8 @@ def histogram(
            raise NotImplementedError()

        plot: Figure = Figure(
-            title=data_type if case is None else f'{data_type} --- {case} case ---',
+            title=f'Histogram of {data_type}'
+            if case is None else f'histogram of {data_type} --- {case} case ---',


Suggested change

if case is None else f'histogram of {data_type} --- {case} case ---',

if case is None else f'Histogram of {data_type} --- {case} case ---',

Signed-off-by: emb4 <[email protected]>

rokamu623

It is not compatible with bokeh 3.x.

rokamu623 · 2023-11-07T03:37:01Z

src/caret_analyze/plot/visualize_lib/bokeh/bokeh.py

-            x_axis_label=x_label, y_axis_label='Probability', width=800
+            title=f'Histogram of {data_type}'
+            if case is None else f'Histogram of {data_type} --- {case} case ---',
+            x_axis_label=x_label, y_axis_label='Probability', plot_width=800


Suggested change

x_axis_label=x_label, y_axis_label='Probability', plot_width=800

x_axis_label=x_label, y_axis_label='Probability', width=800

Signed-off-by: emb4 <[email protected]>

rokamu623

LGTM!

Signed-off-by: emb4 <[email protected]>

rokamu623

LGTM!

xygyo77

LGTM

* Unified graph captions. Signed-off-by: emb4 <[email protected]> * Unified conditional branching of graph captions. Signed-off-by: emb4 <[email protected]> * Fixed flake8 errors. Signed-off-by: emb4 <[email protected]> * Point corrected. Signed-off-by: emb4 <[email protected]> * Corrected string notation. Signed-off-by: emb4 <[email protected]> * Corrected string notation. Signed-off-by: emb4 <[email protected]> * Corrected the points pointed out. Signed-off-by: emb4 <[email protected]> * Corrected caption. Signed-off-by: emb4 <[email protected]> * Corrected caption. Signed-off-by: emb4 <[email protected]> --------- Signed-off-by: emb4 <[email protected]>

* create_response_time_timeseries_plot() API for sim_time. Signed-off-by: ISP akm <[email protected]> * Add support sim_time to histogram APIs. Signed-off-by: ISP akm <[email protected]> * fix: unified graph captions. (#410) * Unified graph captions. Signed-off-by: emb4 <[email protected]> * Unified conditional branching of graph captions. Signed-off-by: emb4 <[email protected]> * Fixed flake8 errors. Signed-off-by: emb4 <[email protected]> * Point corrected. Signed-off-by: emb4 <[email protected]> * Corrected string notation. Signed-off-by: emb4 <[email protected]> * Corrected string notation. Signed-off-by: emb4 <[email protected]> * Corrected the points pointed out. Signed-off-by: emb4 <[email protected]> * Corrected caption. Signed-off-by: emb4 <[email protected]> * Corrected caption. Signed-off-by: emb4 <[email protected]> --------- Signed-off-by: emb4 <[email protected]> * chore(histogram): display number for Y-axis instead of probability (#430) * chore(histogram): display number instead of probability Signed-off-by: takeshi.iwanari <[email protected]> * fix: change hover label Signed-off-by: takeshi.iwanari <[email protected]> --------- Signed-off-by: takeshi.iwanari <[email protected]> * Add sim_time support to create_response_time_stacked_bar_plot() API Signed-off-by: ISP akm <[email protected]> * Histogram legends click policy is set to "hide" Signed-off-by: ISP akm <[email protected]> * Skip invalid records that are involved in sim_time conversion but are invalid. Signed-off-by: ISP akm <[email protected]> * ci(pre-commit): autofix * chore: spell miss coverter->converter Signed-off-by: ISP akm <[email protected]> * fixed pytest errors Signed-off-by: ISP akm <[email protected]> * ci(pre-commit): autofix * fixed pytest errors Signed-off-by: ISP akm <[email protected]> * fixed pytest errors Signed-off-by: ISP akm <[email protected]> * fixed pytest errors Signed-off-by: ISP akm <[email protected]> * fixed pytest errors Signed-off-by: ISP akm <[email protected]> * fixed pytest errors Signed-off-by: ISP akm <[email protected]> * ci(pre-commit): autofix * fixed pytest errors Signed-off-by: ISP akm <[email protected]> * fixed pytest errors Signed-off-by: ISP akm <[email protected]> * add test codes Signed-off-by: ISP akm <[email protected]> * Reflection of PR comments (Part 1) Signed-off-by: ISP akm <[email protected]> * spelling convert->round_convert Signed-off-by: ISP akm <[email protected]> * Supported removal of _convert_timeseries_records_to_sim_time(). Signed-off-by: ISP akm <[email protected]> * ci(pre-commit): autofix * Add sim_time support to StackedBarPlot.to_dataframe. Signed-off-by: ISP akm <[email protected]> * Fixed pytest error. Signed-off-by: ISP akm <[email protected]> * Fixed pytest error. Signed-off-by: ISP akm <[email protected]> * Fixed pytest error. Signed-off-by: ISP akm <[email protected]> * Reflection of PR comments (Part 2) Signed-off-by: ISP akm <[email protected]> * Reflection of PR comments (Part 3) Signed-off-by: ISP akm <[email protected]> --------- Signed-off-by: ISP akm <[email protected]> Signed-off-by: emb4 <[email protected]> Signed-off-by: takeshi.iwanari <[email protected]> Co-authored-by: emori-ctc <[email protected]> Co-authored-by: iwatake <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

This reverts commit b2a3eda.

Unified graph captions.

466e776

Signed-off-by: emb4 <[email protected]>

emori-ctc changed the title ~~feat: Unified graph captions.~~ fix: Unified graph captions. Oct 27, 2023

emori-ctc changed the title ~~fix: Unified graph captions.~~ fix: unified graph captions. Oct 27, 2023

emori-ctc added 2 commits October 30, 2023 09:34

Unified conditional branching of graph captions.

e82a7ee

Signed-off-by: emb4 <[email protected]>

Fixed flake8 errors.

d9653f2

Signed-off-by: emb4 <[email protected]>

rokamu623 self-requested a review October 30, 2023 02:10

rokamu623 requested changes Oct 30, 2023

View reviewed changes

Point corrected.

6493828

Signed-off-by: emb4 <[email protected]>

rokamu623 requested changes Nov 2, 2023

View reviewed changes

Corrected string notation.

d692ff0

Signed-off-by: emb4 <[email protected]>

rokamu623 requested changes Nov 6, 2023

View reviewed changes

emori-ctc added 2 commits November 6, 2023 14:03

Corrected string notation.

a600e5c

Signed-off-by: emb4 <[email protected]>

Corrected string notation.

f98ce94

Signed-off-by: emb4 <[email protected]>

rokamu623 requested changes Nov 7, 2023

View reviewed changes

Corrected the points pointed out.

95d724e

Signed-off-by: emb4 <[email protected]>

rokamu623 previously approved these changes Nov 8, 2023

View reviewed changes

rokamu623 requested a review from isp-uetsuki November 8, 2023 04:34

isp-uetsuki requested review from xygyo77 and removed request for isp-uetsuki November 8, 2023 05:02

Corrected caption.

b6464f5

Signed-off-by: emb4 <[email protected]>

emori-ctc dismissed rokamu623’s stale review via b6464f5 November 10, 2023 04:44

emori-ctc and others added 2 commits November 10, 2023 13:53

Corrected caption.

e0ec807

Signed-off-by: emb4 <[email protected]>

Merge branch 'tier4:main' into unification_caption

f2bacf1

rokamu623 approved these changes Nov 13, 2023

View reviewed changes

xygyo77 approved these changes Nov 13, 2023

View reviewed changes

emori-ctc merged commit b2a3eda into tier4:main Nov 13, 2023
10 checks passed

isp-uetsuki added a commit to isp-uetsuki/CARET_analyze that referenced this pull request Dec 27, 2023

Revert "fix: unified graph captions. (tier4#410)"

beab859

This reverts commit b2a3eda.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: unified graph captions. #410

fix: unified graph captions. #410

emori-ctc commented Oct 27, 2023 •

edited by xygyo77

Loading

rokamu623 left a comment

rokamu623 Oct 30, 2023

rokamu623 Oct 30, 2023

rokamu623 Nov 2, 2023 •

edited

Loading

rokamu623 left a comment

rokamu623 Nov 6, 2023

rokamu623 left a comment

rokamu623 Nov 7, 2023

rokamu623 left a comment

rokamu623 left a comment

xygyo77 left a comment

	title: str = f'Stacked bar of Response Time --- {self._case} case ---'
	title: str = f'Stacked bar of response time --- {self._case} case ---'

		title: str = f'Histogram of {data_type}'if self._case is None else\
		f'histogram of {data_type} --- {self._case} case ---'

	title: str = f'Stacked bar of response Time --- {self._case} case ---'
	title: str = f'Stacked bar of response_time --- {self._case} case ---'

	if case is None else f'histogram of {data_type} --- {case} case ---',
	if case is None else f'Histogram of {data_type} --- {case} case ---',

	x_axis_label=x_label, y_axis_label='Probability', plot_width=800
	x_axis_label=x_label, y_axis_label='Probability', width=800

fix: unified graph captions. #410

fix: unified graph captions. #410

Conversation

emori-ctc commented Oct 27, 2023 • edited by xygyo77 Loading

Description

Related links

Notes for reviewers

Pre-review checklist for the PR author

In-review checklist for the PR reviewers

Post-review checklist for the PR author

rokamu623 left a comment

Choose a reason for hiding this comment

rokamu623 Oct 30, 2023

Choose a reason for hiding this comment

rokamu623 Oct 30, 2023

Choose a reason for hiding this comment

rokamu623 Nov 2, 2023 • edited Loading

Choose a reason for hiding this comment

rokamu623 left a comment

Choose a reason for hiding this comment

rokamu623 Nov 6, 2023

Choose a reason for hiding this comment

rokamu623 left a comment

Choose a reason for hiding this comment

rokamu623 Nov 7, 2023

Choose a reason for hiding this comment

rokamu623 left a comment

Choose a reason for hiding this comment

rokamu623 left a comment

Choose a reason for hiding this comment

xygyo77 left a comment

Choose a reason for hiding this comment

emori-ctc commented Oct 27, 2023 •

edited by xygyo77

Loading

rokamu623 Nov 2, 2023 •

edited

Loading