joaquinanguera / aceR

An R package for processing ACE data
MIT License
3 stars 14 forks source link

Update load-sea-constants-modules.R #5

Closed e-leib closed 5 years ago

e-leib commented 5 years ago

For FRAC_3 module, one of the features of the problems was named (and therefore coded) incorrectly, though the logic was correct. Problems can be congruent or incongruent. A problem is congruent when the larger fraction also has the larger components (e.g., 3/7 > 2/6). A problem is incongruent when the larger fraction has smaller components (e.g., 3/4 > 4/7). In the old code, this was incorrectly named num_larger.

monicathieu commented 5 years ago

Thanks for flagging this! It's not the only reference to this column, so another small change is required to finish this patch. module_fractions_lvl_3() in module-modules-sea.R, the one-summary-per-participant function for this module, currently has a hard-coded expectation for the contents of that congruency column (formerly known as num_larger). If you open up the code, it should be pretty clear what to change. If you're able, please update the variable name references there and then this patch should be clear to merge!

e-leib commented 5 years ago

Just submitted it! Let me know if there is anything else.

Best, Elena

On Mon, Apr 29, 2019 at 6:51 AM Monica Thieu notifications@github.com wrote:

Thanks for flagging this! It's not the only reference to this column, so another small change is required to finish this patch. module_fractions_lvl_3() in module-modules-sea.R, the one-summary-per-participant function for this module, currently has a hard-coded expectation for the contents of that congruency column (formerly known as num_larger). If you open up the code, it should be pretty clear what to change. If you're able, please update the variable name references there and then this patch should be clear to merge!

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/joaquinanguera/aceR/pull/5#issuecomment-487587717, or mute the thread https://github.com/notifications/unsubscribe-auth/AK72JJJL6LNDH6ZUKEQE74TPS34OLANCNFSM4HI7E3OA .

-- Elena Leib PhD Student Building Blocks of Cognition Lab bungelab.berkeley.edu University of California, Berkeley

e-leib commented 5 years ago

Hi Monica,

There is another change that I would like to make. The game coded two of the Fraction Level 3 problems incorrectly for accuracy. Jeci suggested I submit a PR to fix this issue. I have the code ready to go, but I'm not sure if you would like me to do this or if you would like me to show you the code first. I'm also not sure how to make it part of the same PR from before (this is my first time using GitHub!). Let me know what you would like me to do.

Here is the code to put in a mutate function. It seems like it should go in load-sea-constants-modules.R in the FRAC_3 block.

condition.uncorrected = condition, condition = case_when(question_text == "fraction2/6vs3/7" ~ "right", question_text == "fraction3/7vs2/6" ~ "left", TRUE ~ condition.uncorrected), correct_response.uncorrected = correct_response, correct_response = case_when(question_text == "fraction2/6vs3/7" ~ "Right side", question_text == "fraction3/7vs2/6" ~ "Left side", TRUE ~ correct_response.uncorrected), correct_button.uncorrected = correct_button, correct_button = case_when(question_text %in% c("fraction2/6vs3/7", "fraction3/7vs2/6") & correct_button.uncorrected == "correct" ~ "incorrect", question_text %in% c("fraction2/6vs3/7", "fraction3/7vs2/6") & correct_button.uncorrected == "incorrect" ~ "correct", TRUE ~ correct_button.uncorrected)

Basically, the game was coding 2/6 as greater than 3/7, so for these two questions, condition, correct_response, and correct_button need to be reversed (if it was left, now needs to be right, etc.).

Best, Elena

On Tue, Apr 30, 2019 at 1:39 PM Monica Thieu notifications@github.com wrote:

Merged #5 https://github.com/joaquinanguera/aceR/pull/5 into master.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/joaquinanguera/aceR/pull/5#event-2310676177, or mute the thread https://github.com/notifications/unsubscribe-auth/AK72JJNF53XJPEQIJ2YO6NLPTCVBLANCNFSM4HI7E3OA .

-- Elena Leib PhD Student Building Blocks of Cognition Lab bungelab.berkeley.edu University of California, Berkeley

monicathieu commented 5 years ago

Hi Elena,

You can make a separate pull request for this (since it's solving a slightly different problem than the last pull request you made). You can insert the code you've written above into the relevant .R document on your personal fork, and then submit the PR, and then I can use GitHub's built-in code review tools to make specific code suggestions if I see anything to revise.

thanks for staying on top of updating the SEA modules! glad to have your expertise on board.