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

Improved the Describe > One Variable > Summarise dialog #7410

Merged
merged 6 commits into from
Apr 29, 2022

Conversation

Ivanluv
Copy link
Contributor

@Ivanluv Ivanluv commented Apr 25, 2022

fixes #7081 and replaces PR #7123

@africanmathsinitiative/developers this is ready for review

@lloyddewit
Copy link
Contributor

@HawardKetoyoMsatsi Please could you review?
@rdstern Please could you test?
Thanks!

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.

@Ivanluv and @lilyclements this is excellent now. All over except the shouting! I used the usual survey data and could get all I wanted in terms of the summary statistics.
Just some cosmetic things remain. I think:
a) Is the terminology the wrong way round? The default is Display summaries as rows. I think you mean columns? When I make both summaries and variables as rows, then I only get one row and many columns. So maybe all you need to change is the label.
b) When I untick both, there is just a single column. And there is no labelling, just the numbers.
c) There is the margin All, over which I have no control. In my examples it just gives the total count. I'm not sure what that means and am not sure I want the margin here?

@Ivanluv
Copy link
Contributor Author

Ivanluv commented Apr 26, 2022

@rdstern could you have a look again?

Copy link
Contributor

@HawardKetoyoMsatsi HawardKetoyoMsatsi left a comment

Choose a reason for hiding this comment

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

@ivan I requested for a few changes, please take a look.

instat/dlgOneVariableSummarise.vb Outdated Show resolved Hide resolved
instat/dlgOneVariableSummarise.vb Show resolved Hide resolved
instat/dlgOneVariableSummarise.vb Outdated Show resolved Hide resolved
instat/dlgOneVariableSummarise.vb Show resolved Hide resolved
instat/dlgOneVariableSummarise.vb Show resolved Hide resolved
@Ivanluv
Copy link
Contributor Author

Ivanluv commented Apr 28, 2022

@HawardKetoyoMsatsi ?

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.

@Ivanluv and @lilyclements - great!

@lloyddewit
Copy link
Contributor

@HawardKetoyoMsatsi ?

@HawardKetoyoMsatsi Thank you for your review. I went through the comments/responses and set the comments to resolved. If I misunderstood anything, then let me know.
Thanks

Copy link
Contributor

@lloyddewit lloyddewit left a comment

Choose a reason for hiding this comment

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

@Ivanluv Thank you! looks good

@lloyddewit lloyddewit dismissed HawardKetoyoMsatsi’s stale review April 29, 2022 10:18

I consider all comments resolved

@lloyddewit lloyddewit changed the title Improvements to the One Variable Summaries Improved the Describe > One Variable > Summarise dialog Apr 29, 2022
@lloyddewit
Copy link
Contributor

@Ivanluv does this PR fully resolve #7081? Or just partially?

@lloyddewit lloyddewit merged commit 1d62394 into IDEMSInternational:master Apr 29, 2022
@Ivanluv
Copy link
Contributor Author

Ivanluv commented May 4, 2022

@lloyddewit this fully fixes #7081

@Ivanluv Ivanluv deleted the New_One_variable#7081 branch July 6, 2022 08:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve the Describe > One Variable > Summarise dialogue and possibly 2-variable and 3-variable?
4 participants