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.

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

Improvements in Describe > Graphs > Boxplot dialog #8766

Merged
merged 3 commits into from
Jan 26, 2024

Conversation

Vitalis95
Copy link
Contributor

Fixes Partially #8758
@rdstern , @N-thony , The PR adds'legend'and 'facets' options to the boxplot option in the Boxplot dialog

@Vitalis95 Vitalis95 changed the title Improvements in Describe > Graphs > Boxplot Improvements in Describe > Graphs > Boxplot dialog Jan 25, 2024
N-thony
N-thony previously approved these changes Jan 26, 2024
Copy link
Collaborator

@rdstern rdstern left a comment

Choose a reason for hiding this comment

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

@Vitalis95 this is brilliant. I tried with both boxplots and with the general. Both work and I note that both also set the control in the sub-dialog - which is great.
Could you add the two controls, identically in the jitter and violin plot options. Then we can merge. And later they need to go into the other graph dialogs.
@Vitalis95 and @N-thony I assume it is difficult to code "the other way round"? If I make the change in the sub-dialog, then I didn't expect it to reflect automatically in the main dialog. But I wondered if it might set the main dialog when the boxplot dialog is re-opened?

@N-thony
Copy link
Collaborator

N-thony commented Jan 26, 2024

@Vitalis95 this is brilliant. I tried with both boxplots and with the general. Both work and I note that both also set the control in the sub-dialog - which is great. Could you add the two controls, identically in the jitter and violin plot options. Then we can merge. And later they need to go into the other graph dialogs. @Vitalis95 and @N-thony I assume it is difficult to code "the other way round"? If I make the change in the sub-dialog, then I didn't expect it to reflect automatically in the main dialog. But I wondered if it might set the main dialog when the boxplot dialog is re-opened?

I don't understand that, isn't it a bit of complication?. When you make the change on the sub dialogue, it should reflect directly in the main dialogue. Why it should reflect after re-opening the dialogue?

@rdstern
Copy link
Collaborator

rdstern commented Jan 26, 2024

@N-thony you asked " don't understand that, isn't it a bit of complication?. When you make the change on the sub dialogue, it should reflect directly in the main dialogue. Why it should reflect after re-opening the dialogue?"

That's because I was wrong. I assumed it would be difficult to sybchronise the two controls completely. I had only checked the consistency from main dialog to sub-dialog, after re-opening the dialog. I now see that going from main dialog to sub-dialog it immediate - they are really synchronised. So my new request is whether this synchronisation could be "both ways". That would be perfect.

Is it a feature of the control that can be done generally. I now checked with the horizontal plot checkbox. This is on most of the graph dialogs. That also has "one-way" synchronisation. So the change in the status of the control goes from the main to the sub-dialog - under coordinates in the sub-dialog, but not back again.

This is also showing the benefits of writing the help - which is why I am here. Why didn't I do that earlier?

However wew are making good progress. I claim we are getting to a system where the ease of use of ggplot is becoming excellent.

@Vitalis95
Copy link
Contributor Author

@rdstern , the change I made is adding the two controls in the jitter and violin plot options

Copy link
Collaborator

@rdstern rdstern left a comment

Choose a reason for hiding this comment

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

@Vitalis95 excellent, including the way you explain exactly what you changed, so I know what to check.
@N-thony I am still keen on 2-way, on adding these controls to the other graph types and on further improvements to the boxplot dialog. But I also suggest we merge as we progress. So I am approving so you can check and merge if ok.

@N-thony N-thony merged commit ae4821b into IDEMSInternational:master Jan 26, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants