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

Unit testing, renaming functions, adding docs #124

Open
wants to merge 68 commits into
base: main
Choose a base branch
from
Open

Conversation

RayStick
Copy link
Member

@RayStick RayStick commented Sep 13, 2024

Closes #38

Overall description

This PR adds unit testing for this R package.

In order to do this, the main package function(s) needed to be split up into smaller functions, in order to test. When writing tests, it was sometimes obvious that functions should be improved in various ways, and a few sub-optimal coding things were found and corrected.

It also became obvious that there were 2 parts to this package as a whole - the browsing and the mapping. This has been made clearer now, by splitting up two functions, and adding some nice plots.

Changes implemented (by directory)

Parent directory:

  • Updated the .Rbuildignore and .gitignore files to better reflect the package structure
    • there were 3 .gitignore files and I have now combined into 1
  • DESCRIPTION and NAMESPACE reflect changes to the package, specifically to do with how imports and exports are handled
  • README.md reflects the new code changes

R directory:

There are now 4 main functions that a user can interact with:

  1. browseMetadata.R (new!)
  2. mapMetadata.R(previously domain_mapping.R)
  3. mapMetadata_compare_outputs.R (previously compare_sessions.R)
  4. mapMetadata_convert_outputs.R(previously convert_outputs.R)

All of the other files in the R directory are either:

  1. Sub-functions that are called within one of the 4 functions above
  2. Package data

data directory:

The same 2 dataframes were made multiple times across the functions. Therefore, to reduce lines of code, they have now been included in the package data:

  • data/log_Output.rda
  • data/Output.rda

These can now be read into the functions with one line e.g. Output <- get("Output")

inst directory:

Some example inputs and outputs have been moved or added, so they can be referenced in the README, or other documentation throughout the package.

man directory:

All new functions require documentation via .Rd files

tests directory:

All functions require unit tests, written with the testthat package.

Checklist to make it ready for review (for @RayStick):

  • The title of this PR is clear and self-explantory.
  • I added any appropriate labels to this PR.
  • Appropriately handle imports across /R and /test directory
  • Split browseMetadata.R into smaller functions?
  • Ensure all unit tests pass
  • Do user testing to ensure all functions that user will interact with still work as intended - write notes for reviewer as I go
  • Ensure README reflect the new code changes
  • Ensure devtools::check() is still happy

Checklist for reviewers (@Rainiefantasy )

Please feel free comment on my PR while it's a draft and give me feedback on the development!

  • Accept Rachael's apology for how long this PR is - we can have a 1:1 to bring context before you review
  • First read the new README file for an overview of what has changed from user perspective
  • Look at all the changed files (high-level) using the ' Changes implemented (by directory)' section above as a guide
  • Complete user testing of all 4 functions to ensure all functions work as intended and match the README guide (see below)

Tips for user testing (@Rainiefantasy)

  1. Open up R Studio with nothing in your env etc.
  2. setwd('your-path/test_dir')
  3. remove.packages("browseMetadata") - you may need to specify a path
  4. devtools::install_github("aim-rsf/browseMetadata", ref = 'big-refactor')
  5. library(browseMetadata)

Testing browseMetadata.R

  • First run as the README suggests (https://github.com/aim-rsf/browseMetadata/blob/big-refactor/README.md#browsemetadatar) using package files and no outputdir
  • Continue to use package files, but change the output_dir
  • Then use some different inputs files e.g. ADBE and EDUW datasets
  • For each run above, check there are (1) 3 file outputs (2) they are in the output dir you expect and (3) the html outputs can be opened in the browser and they look sensible

Testing mapMetadata.R

  • First run in demo mode
    • Process 1 table in a run, delete outputs
    • Process 1 table in a run, keep outputs
    • Process 2 tables in a run, check that the COPY function kicks in
  • Then run outside of demo mode (changing the input arguments in various ways to ensure function still works)
  • This is the main function to test (the other 3 are much simpler) so please test it with many variations :)

Testing mapMetadata_convert_outputs.R

  • This function hasn't changed so only a quick test should do the trick:

mapMetadata_convert_outputs(output_csv = 'OUTPUT_xxx.csv', output_dir = /path/test_dir/')

Check that the above call outputs L-OUTPUT_xxx.csv and that any rows that had multiple categorizations have now been split onto their own rows.

Testing mapMetadata_compare_outputs.R

  • This function has only changed a little but a quick test should do the trick
  • Remember you have the files in the inst/inputs folder to point towards as quick inputs for some of the arguments

@github-actions github-actions bot added the documentation Improvements or additions to documentation label Sep 13, 2024
@RayStick RayStick changed the title big refactor - renaming and adding tests Unit testing, renaming functions, adding docs Sep 13, 2024
@RayStick RayStick added the enhancement Feature improvement or addition label Sep 13, 2024
@RayStick RayStick added this to the before rOpenSci milestone Sep 13, 2024
@aim-rsf aim-rsf deleted a comment from allcontributors bot Sep 23, 2024
@RayStick RayStick marked this pull request as ready for review September 27, 2024 16:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement Feature improvement or addition internal Changes related to GH workflows, actions, apps
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create a test for the function with testthat
1 participant