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

Optimize, standardize, and add tests and coverage reports #47

Merged
merged 27 commits into from
Nov 29, 2018
Merged

Optimize, standardize, and add tests and coverage reports #47

merged 27 commits into from
Nov 29, 2018

Conversation

patrickbarks
Copy link
Collaborator

(1) Favour slot access with @ internally
It's substantially faster. Accessors aren't really slowing down any of our functions, but particularly for 'lower-level' functions that may be called repeatedly in users' loops or vectorized functions (e.g. [, $, matA()), I think it's a good idea to optimize.

(2) Add methods for all mat-related accessors (matA(), matU(), ..., matrixClass(), etc.) for signature 'list', to enable e.g. matA(db$mat). The purpose is to make it easier to work with the tidyverse, e.g.

db_tidy <- fetchDB("Compadre") %>% 
  CompadreData() # extract data slot (just a tibble)

db_tidy %>% 
  mutate(stage_surv = lapply(matU(mat), colSums)) # use matU on list-column


# without the list method, could only use matU() within a custom fn, e.g.
db_tidy %>% 
  mutate(stage_surv = lapply(mat, function(x) colSums(matU(x))))

(3) Re-incorporate @levisc8's unit tests, and expand to all fns (toward #13). Also added code coverage checks via covr, to check how much of our codebase is covered by unit tests. Results will be shown in badge on README. Will also give coverage reports for pull requests.

Other edits:

  • remove convertLegacyDB and replace use with asCompadreDB (they do the same thing)
  • add CompadreLegacy subsample to distribute with package (for examples and testing asCompadreDB)
  • further optimization to validCompadreMat checks (down to 3.5 s for COMPADRE X.X.X)
  • edit [ method to allow column drop with db[,-j] (still always retains mat column)
  • add examples to ?CompadreDB doc
  • update and standardize documentation
  • change data.R doc to raw markdown to enable better formatting (in my opinion)
  • revert slot names to data and version (per my suggesion in rename "data" generic? #37)
  • added Contributor Code of Conduct (required by rOpenSci, and good idea generally)

… accessors ~20% faster), and remove accessors stageNames and stageStatus (same functionality as MatrixClassOrganized and MatrixClassAuthor)
…ct slot access through @ b/c it's substantially faster
@jonesor jonesor merged commit 0c99ba9 into jonesor:devel Nov 29, 2018
@jonesor
Copy link
Owner

jonesor commented Nov 29, 2018

Thanks @patrickbarks. Looks great!

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.

2 participants