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

Update README #174

Closed
wants to merge 19 commits into from
Closed

Update README #174

wants to merge 19 commits into from

Conversation

iuliadmtru
Copy link
Contributor

@iuliadmtru iuliadmtru commented Jan 11, 2023 •
edited
Loading

  • The references to the old design were removed. All examples now use the new general design.
  • The Basic usage section is now more complete.
  • I made some rephrasing in the other sections.
  • The Documentation section was removed. It is now referenced at the end of the Basic usage section.

@iuliadmtru
Copy link
Contributor Author

This partially solves #173.

ayushpatnaikgit
ayushpatnaikgit reviewed Jan 11, 2023
View reviewed changes
README.md Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

codecov-commenter commented Jan 11, 2023 •
edited
Loading

Codecov Report

Merging #174 (c42acc7) into singledesign (7e946cc) will increase coverage by 20.99%.
The diff coverage is 85.71%.

@@                Coverage Diff                @@
##           singledesign     #174       +/-   ##
=================================================
+ Coverage         64.79%   85.79%   +20.99%     
=================================================
  Files                13       13               
  Lines               196      176       -20     
=================================================
+ Hits                127      151       +24     
+ Misses               69       25       -44
Impacted Files Coverage Δ
src/SurveyDesign.jl 92.30% <ø> (ø)
src/bootstrap.jl 95.65% <ø> (ø)
src/boxplot.jl 100.00% <ø> (ø)
src/by.jl 100.00% <ø> (ø)
src/hist.jl 91.66% <ø> (ø)
src/jackknife.jl 0.00% <ø> (ø)
src/mean.jl 100.00% <ø> (ø)
src/ratio.jl 100.00% <ø> (ø)
src/total.jl 100.00% <ø> (ø)
src/show.jl 88.88% <85.71%> (+88.88%) ⬆️
... and 1 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

smishr
smishr approved these changes Jan 11, 2023
View reviewed changes
Copy link
Contributor

@smishr smishr left a comment

Choose a reason for hiding this comment

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

yes looks good.

ayushpatnaikgit
ayushpatnaikgit reviewed Jan 11, 2023
View reviewed changes
README.md Outdated

This package is used to study complex survey data. It aims to be a fast alternative to the [Survey package in R](https://cran.r-project.org/web/packages/survey/index.html) developed by [Professor Thomas Lumley](https://www.stat.auckland.ac.nz/people/tlum005).
This package currently supports simple random sample, stratified sample, single and
Copy link
Member

Choose a reason for hiding this comment

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

It supports any kind of sample.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would a stratified + cluster sample give correct results?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ayushpatnaikgit What do you think now?

ayushpatnaikgit
ayushpatnaikgit reviewed Jan 11, 2023
View reviewed changes
README.md Outdated Show resolved Hide resolved
@ayushpatnaikgit
Copy link
Member

It seems redundant to show mean, domain estimation etc for all the design. Perhaps show the constructor for srs, stratified sample, and cluster sample, but estimation for only 1.

@iuliadmtru @ayushpatnaikgit
Rephrase
7bfd636 
Co-authored-by: Ayush Patnaik <[email protected]>
@iuliadmtru
Copy link
Contributor Author

It seems redundant to show mean, domain estimation etc for all the design. Perhaps show the constructor for srs, stratified sample, and cluster sample, but estimation for only 1.

Ok, I agree.

@iuliadmtru
Copy link
Contributor Author

I suggest merging #175 before merging this. The README.md file was not changed in that PR so there will be no conflicts. I can pull the changes from there to here and update the README according to the new show method.

@ayushpatnaikgit
Copy link
Member

This will need to change according to PR #175

ayushpatnaikgit
ayushpatnaikgit reviewed Jan 13, 2023
View reviewed changes
README.md Outdated Show resolved Hide resolved
@iuliadmtru iuliadmtru mentioned this pull request Jan 13, 2023
Merged
@iuliadmtru
Copy link
Contributor Author

Since this PR includes commits from the previously merged one (#175), I am closing it and creating a proper PR for the README update.

@iuliadmtru iuliadmtru closed this Jan 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@smishr smishr smishr approved these changes

@ayushpatnaikgit ayushpatnaikgit ayushpatnaikgit approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@iuliadmtru @codecov-commenter @ayushpatnaikgit @smishr