Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Mark/1488 customize histogram calculations #2141

Merged
merged 54 commits into from
May 2, 2023
Merged

Mark/1488 customize histogram calculations #2141

merged 54 commits into from
May 2, 2023

Conversation

markccchiang
Copy link
Contributor

@markccchiang markccchiang commented Apr 14, 2023 •
edited by kswang1029
Loading

Description

This customized the histogram calculations as described in #1488. The companion backend PR and protobuf PR are shown on the hyperlinks.

Checklist

For linked issues (if there are):

  • assignee and label added
  • ZenHub issue connection, board status, and estimate updated

For the pull request:

  • reviewers and assignee added
  • ZenHub estimate, milestone, and release (if needed) added
  • e2e test passing / corresponding fix added
  • changelog updated / no changelog update needed
  • protobuf updated to the latest dev commit / no protobuf update needed
  • BackendService unchanged / BackendService changed and corresponding ICD test fix added

@markccchiang
Add more variables in the SetHistogramRequirements message
6d7b0a4
@markccchiang
Add default pixel bounds in the histogram request message
4bc624b
@markccchiang
Merge branch 'dev' of https://github.com/CARTAvis/carta-frontend into…
a6db935 
… mark/custom_histogram_calculations_test
@markccchiang
Merge branch 'dev' of https://github.com/CARTAvis/carta-frontend into…
cbe0c5b 
… mark/custom_histogram_calculations_test
@markccchiang
Add a histogram config setting panel
ce3f174
@markccchiang
Add a handler for histogram generation with new config settings
714a153
@markccchiang
Update the protobuf with the new dev branch
317a7f4
@markccchiang
Merge branch 'dev' of https://github.com/CARTAvis/carta-frontend into…
81fc407 
… mark/custom_histogram_calculations_test
@markccchiang
Modify the HistogramConfig message and add it to the RegionHistogramData
25b9fe1
@markccchiang
Add histogram config setting to the protobuf message SetHistogramRequ…
c2453c0 
…irements
@markccchiang
Only update the channel histogram data if its configs for number of b…
9f5c11e 
…ins and pixel bounds are not fixed
@markccchiang
Render the correct histogram plot with respect to its configurations …
f2d896e 
…on the wedget
@markccchiang
Add a reset button to reset histogram configurations
bad55d3
@markccchiang
Automatically generate a new histogram when new configurations are set
e32f1b0
@markccchiang
Refactor the HistogramConfigPanelComponent
1831eec
@markccchiang
Change the format of histogram config panel
663b106
@markccchiang
Cache histogram config settings from automatically generated values
5244992
@markccchiang
Change the format of setting histogram number of bins panel
4532dd8
@markccchiang
Fix the code style
74ca4d1
@markccchiang
Merge branch 'dev' of https://github.com/CARTAvis/carta-frontend into…
39f779d 
… mark/custom_histogram_calculations_test
@markccchiang
Modify the tool tip text for the setting of maximum slider value
215116f
@markccchiang
Change the name of title for histogram configuration panel
d5bf354
@markccchiang
Set the lower bound of number of bins as 2
54f0be1
@markccchiang
Redefine the equality of two numbers
a14e621
@markccchiang
Remove the bin width variable from histogram configuration message
81971ea
@markccchiang markccchiang mentioned this pull request Apr 14, 2023
Merged
5 tasks
@markccchiang
Copy link
Contributor Author

I am fixing a bug found by @YuHsuan-Hwang. And also changing the codes according to suggestions. Will push new commits when they are done.

@kswang1029 kswang1029 self-requested a review April 26, 2023 12:09
YuHsuan-Hwang
YuHsuan-Hwang requested changes Apr 27, 2023
View reviewed changes
Copy link
Collaborator

@YuHsuan-Hwang YuHsuan-Hwang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes are looking good! I have some feedback for the details:

src/components/Histogram/HistogramComponent.tsx Outdated Show resolved Hide resolved
src/components/Histogram/HistogramComponent.tsx Outdated Show resolved Hide resolved
src/components/Histogram/HistogramSettingsPanelComponent/HistogramConfigPanelComponent.tsx Outdated Show resolved Hide resolved
src/components/Histogram/HistogramSettingsPanelComponent/HistogramConfigPanelComponent.tsx Outdated Show resolved Hide resolved
kswang1029
kswang1029 approved these changes Apr 28, 2023
View reviewed changes
Copy link
Collaborator

@kswang1029 kswang1029 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looking good and no regression from e2e tests. 👍

@YuHsuan-Hwang
Copy link
Collaborator

Added fix for #2150.

@YuHsuan-Hwang YuHsuan-Hwang merged commit 37e1513 into dev May 2, 2023
@YuHsuan-Hwang YuHsuan-Hwang deleted the mark/1488_customize_histogram_calculations branch May 2, 2023 14:38
@kswang1029 kswang1029 mentioned this pull request May 2, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@YuHsuan-Hwang YuHsuan-Hwang YuHsuan-Hwang approved these changes

@kswang1029 kswang1029 kswang1029 approved these changes

@TienHao TienHao Awaiting requested review from TienHao

Assignees

@YuHsuan-Hwang YuHsuan-Hwang

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@markccchiang @kswang1029 @acdo2002 @YuHsuan-Hwang