Add test for reverse_items

[strengejacke]

No description provided.

[gemini-code-assist]

Summary of Changes

Hello @strengejacke, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request primarily focuses on improving the get_scores function by enhancing its test coverage and adding a new user-facing option. The changes ensure the robustness of the reverse_items feature and provide more control over function output, contributing to a more stable and user-friendly experience.

Highlights

• Testing: I've added new unit tests for the reverse_items functionality within the get_scores function. These tests cover both the successful reversal of specified items and the expected warning behavior when non-existent items are provided for reversal.
• Function Enhancement: I've introduced a new verbose parameter to the get_scores function. This parameter, defaulting to TRUE, allows users to control whether messages related to item reversal are printed to the console.
• Documentation Update: The documentation for the get_scores function, including its Roxygen comments and manual page (man/get_scores.Rd), has been updated to reflect the addition of the new verbose parameter.
• Version Bump: The package version in the DESCRIPTION file has been incremented from 0.26.0.10 to 0.26.0.11.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request adds tests for the reverse_items argument in the get_scores function, which is a valuable addition for ensuring code correctness. It also introduces a new verbose parameter to control messaging. The implementation looks good, but I've pointed out that the new verbose parameter is not covered by tests. I've provided a suggestion to add tests for this new functionality to ensure it behaves as expected.

[DominiqueMakowski]

@profandyfield @strengejacke I'd really reconsider including an argument for reversing items:

• We are encouraging bad practices
• People usually later still need to compute averages / sums of dimensions so they will need to reverse the items explicitly, so they might as well do it before, especially since we have a nice datawizard::reverse_scale() function
• I remember teaching (in QQM) and for some of the psych functions the tutorial explained to students that they need to pass "a vector of 1 and -1 corresponding to the index of the variables to reverse" and I can tell you this was SUPER confusing to students, 1) it relies on variables order which is dangerous and bad practice, 2) if you're unfamiliar with programming, a vector of ones and minus ones is not transparent at all

So i'd remove this argument from our functions

[profandyfield]

You have to do what's right for packages/ethos, but if you're going to remove it can you please do it quickly otherwise my chapter will be out of date before it's even copy-edited! If you remove this fast I still have a few days to rewrite things (again!)

[DominiqueMakowski]

(regardless of removing the argument, rewrite your chapter to reverse explicitly, the small rewriting pain now will be outweighed by the pleasure of doing what's right) [source: trust me bro] 😬

[strengejacke]

People usually later still need to compute averages / sums of dimensions so they will need to reverse the items explicitly, so they might as well do it before, especially since we have a nice datawizard::reverse_scale() function

We also have this item in get_scores(), and if you do it once in factor_analysis() or principal_components(), the "reversed" data is stored as attributes, for further computation.

But I have no string opinion here, we can have both. One argument against having a reverse_item argument in that people have to remember in subsequent processing whether they're are working with the reversed data set or not, which is not that explicit when you

1. input "not reversed" data to factor_analysis()
2. factor_analysis() stores the "reversed" data as attribute
3. Further functions like get_scores() or check_itemscale() applied on the object returned by factor_analysis() use this already "reversed" data, so no need to use the reverse_items argument in those post-processing functions again (if you do it, you double reversed, meaning you have the "unreversed" data again)

[strengejacke]

Although: we could also save reverse_items as attribute, checking these in other functions. This would avoid double-reversing.

[DominiqueMakowski]

Although: we could also save reverse_items as attribute, checking these in other functions

feature creep alert

[profandyfield]

I think if the package ethos is 'always preprocess no matter what' then strip it out of everything. Having both is (a) over complicated/risks inconsistency; (b) potentially confusing. I'll work with whatever you decide to do I just don't want to work on the basis of something that changes in 2 weeks time.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 62.48%. Comparing base (a7547a9) to head (9921120).
Report is 237 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1122      +/-   ##
==========================================
+ Coverage   59.80%   62.48%   +2.68%     
==========================================
  Files         188      193       +5     
  Lines       14229    14817     +588     
==========================================
+ Hits         8509     9258     +749     
+ Misses       5720     5559     -161     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.