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

Mixed Structure Data Loss Bug Resolution #201

Merged
merged 8 commits into from
Sep 3, 2024
Merged

Conversation

rsh52
Copy link
Collaborator

@rsh52 rsh52 commented Aug 30, 2024

Description

This PR addresses a bug found where mixed structure databases outputs may result in data loss in instances where a form may be both "repeating separately" and "repeating together".

Side note: Are there any additional things we should add to the pkgdown site?

Proposed Changes

List changes below in bullet format:

  • Update convert_mixed_structure() field under clean_redcap_long() to account for missing case
  • Update associated mixed structure test to account for case
  • Update the Mixed Structure test REDCap with a repeating-together event
  • Update NEWS.md to be more accurate to present repo

Screenshots
If applicable, add screenshots to help explain the proposed changes

Issue Addressed

Closes #200

PR Checklist

Before submitting this PR, please check and verify below that the submission meets the below criteria:

  • New/revised functions have associated tests
  • [NA] New/revised functions that update downstream outputs have associated static testing files (.RDS) updated under inst/testdata/create_test_data.R
  • New/revised functions use appropriate naming conventions
  • New/revised functions don't repeat code
  • Code changes are less than 250 lines total
  • Issues linked to the PR using GitHub's list of keywords
  • The appropriate reviewer is assigned to the PR
  • The appropriate developers are assigned to the PR
  • [NA] Pre-release package version incremented using usethis::use_version()
    • Using the same version for now under an upcoming 1.2.0 release

Code Review

This section to be used by the reviewer and developers during Code Review after PR submission

Code Review Checklist

  • I checked that new files follow naming conventions and are in the right place
  • I checked that documentation is complete, clear, and without typos
  • I added/edited comments to explain "why" not "how"
  • I checked that all new variable and function names follow naming conventions
  • I checked that new tests have been written for key business logic and/or bugs that this PR fixes
  • I checked that new tests address important edge cases

@rsh52 rsh52 added the bug Something isn't working label Aug 30, 2024
@rsh52 rsh52 self-assigned this Aug 30, 2024
@rsh52 rsh52 changed the title Mixed structure rt rs bug Mixed Structure Data Loss Bug Resolution Aug 30, 2024
@rsh52 rsh52 marked this pull request as ready for review August 30, 2024 17:12
@rsh52 rsh52 requested a review from ezraporter August 30, 2024 17:12
Copy link
Collaborator

@ezraporter ezraporter left a comment

Choose a reason for hiding this comment

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

Noted a couple issues in the comments but I think the idea is right!

R/clean_redcap_long.R Outdated Show resolved Hide resolved
R/clean_redcap_long.R Outdated Show resolved Hide resolved
R/clean_redcap_long.R Outdated Show resolved Hide resolved
R/clean_redcap_long.R Outdated Show resolved Hide resolved
@rsh52 rsh52 requested a review from ezraporter August 30, 2024 19:32
Copy link
Collaborator

@ezraporter ezraporter left a comment

Choose a reason for hiding this comment

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

Looks good now! Thanks for making those changes

@rsh52 rsh52 merged commit 9342e58 into main Sep 3, 2024
4 checks passed
@rsh52 rsh52 deleted the mixed_structure_rt_rs_bug branch September 3, 2024 13:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Data loss for repeating together events when instruments are additionally mapped to other event types
2 participants