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

Hw9 is ready for grading #9

Open
sepkamal opened this issue Nov 29, 2017 · 2 comments
Open

Hw9 is ready for grading #9

sepkamal opened this issue Nov 29, 2017 · 2 comments

Comments

@sepkamal
Copy link
Owner

sepkamal commented Nov 29, 2017

Fingers crossed the package installation goes well for you guys!

Here is the homework assignment: https://github.com/sepkamal/STAT545-hw-Kamal-Sepehr/tree/master/Hw09

You can use this code: devtools::install_github("sepkamal/powers") to install my package

@qiaoyuet
Copy link

qiaoyuet commented Dec 1, 2017

Peer Review:

Functions & Document:

  • It's nice to have a plot_it argument in the pow function. And good method to handle NAs.
  • Not quite understand why there's no function inside negtenvec and tenvec.
  • I think it'll be more informative to give the arguments more specific names. For example in the pow function, a can be changed to power?
  • Document for power_datatable is very nicely written.
  • Didn't understand about defining pipe operator, you didn't seem to use it inside the power_datatable function?
  • Package passed check with no errors, warnings and notes.

Tests:

  • It's good to have a test that checks whether logicals automatically convert to numeric.
  • I think it would be better if you write the test for power_datatable in another test_file?
  • And it will be good to write more tests for each functions in the R folder.

Readme and vignettes:

  • Nice to indicate how to install this package.
  • I think it'll be better if you include more examples, maybe one example for each function? Even though some of them are written as internal functions, just for purpose of practicing, it would be better to include examples too?
  • Vignettes are nicely written and gives different contents from readme.
  • I'm not sure what are the consequences of not knitting vignettes.Rmd, but usually knitting to html will make it easier to view if someone called browseVignettes("your_package") to view the vignette.
    Overall, great work!

@KateJohnson
Copy link
Collaborator

Hi @sepkamal

Great job! Really nice functions, I found them all useful, particularly the character vectors and NAs. Nice ideas!

  • Thanks for the help on viewing the vignette, I couldn’t figure out how to do that when vignette() didn’t work
  • Your vignette was very well organized. I liked that each function had its own page and that you specified all the arguments and gave examples. It might be more useful to name the arguments something other than a and x though.
  • I didn’t see your tests in the testthat folder. Maybe I missed them in your repo?

👍

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

No branches or pull requests

3 participants