Skip to content

Conversation

@strengejacke
Copy link
Member

Fixes #655

@strengejacke
Copy link
Member Author

@etiennebacher Maybe we can move the code to create the crosstable into an own function, and then simplify (and only do) the formatting in format(). Then we have the proportions accessible as attribute and can maybe add an as.prop.table() method or something.

@etiennebacher
Copy link
Member

Sorry, I forgot about this one. I'll make a CRAN release this week so it would be nice to have this PR in it too.

Maybe we can move the code to create the crosstable into an own function, and then simplify (and only do) the formatting in format(). Then we have the proportions accessible as attribute and can maybe add an as.prop.table() method or something.

Sounds good to me but isn't this already the case since the crosstable is built in .crosstable()?

@strengejacke
Copy link
Member Author

The proportions are currently added in the format() method. In this PR, I wrote a .prop_table() function that calculates proportions and adds them as attributes, so we can easily extract then. We should built upon this function in format(). to avoid duplicated code .

@strengejacke
Copy link
Member Author

strengejacke commented Oct 2, 2025

Sorry, when updating the RD, other docs were also updated due to roxygen update.

I also got this warning:

@strengejacke strengejacke requested a review from Copilot October 2, 2025 07:56
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR implements a feature to extract proportions from crosstables generated by data_tabulate(). The main purpose is to add functionality that saves proportion calculations as attributes and provides a new as.prop.table() method for easy access to these proportions.

  • Adds as.prop.table() method for extracting proportions from crosstables
  • Refactors proportion calculation code to eliminate duplication and improve maintainability
  • Updates documentation examples to use simplified withAutoprint() syntax

Reviewed Changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
R/data_tabulate.R Adds new as.prop.table() method and S3 methods for crosstab objects
R/data_xtabulate.R Refactors proportion calculation logic and adds .prop_table() helper function
man/data_tabulate.Rd Updates documentation to describe the new as.prop.table() method
NAMESPACE Exports the new as.prop.table() method and S3 methods
NEWS.md Documents the new feature in the changelog
DESCRIPTION Updates package version and RoxygenNote version
Various .Rd files Updates documentation examples to use simplified syntax
R/utils.R Minor comment formatting fix

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@codecov
Copy link

codecov bot commented Oct 3, 2025

Codecov Report

❌ Patch coverage is 58.13953% with 36 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.64%. Comparing base (9a624b1) to head (4ff1f54).
⚠️ Report is 27 commits behind head on main.

Files with missing lines Patch % Lines
R/data_tabulate.R 0.00% 33 Missing ⚠️
R/data_xtabulate.R 95.91% 2 Missing ⚠️
R/utils.R 75.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #656      +/-   ##
==========================================
- Coverage   91.73%   91.64%   -0.10%     
==========================================
  Files          76       76              
  Lines        6410     6604     +194     
==========================================
+ Hits         5880     6052     +172     
- Misses        530      552      +22     

☔ 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.

@etiennebacher
Copy link
Member

etiennebacher commented Oct 8, 2025

@strengejacke I see that tests are passing, is this ready for review?

@strengejacke strengejacke marked this pull request as ready for review October 9, 2025 04:58
@strengejacke
Copy link
Member Author

Ah, yes!

@etiennebacher
Copy link
Member

etiennebacher commented Oct 9, 2025

@strengejacke I've pushed a commit to move as.prop.table() to its own Rd page because the argument x is present in data_tabulate() and as.prop.table() but doesn't mean the same thing between these functions.

Before, ?as.prop.table would lead to the documentation of data_tabulate() that said:

x: A (grouped) data frame, a vector or factor.

which isn't true for as.prop.table(). Let me know if you're ok with that. Also, the docs said that this is only valid on data_tabulate() calls that have by, but it seems that proportions is also required?

library(datawizard)

data(efc)

data_tabulate(efc, select = "e42dep", by = "c172code") |> 
  as.prop.table()
#> Warning: No proportions available.
#> NULL

data_tabulate(efc, select = "e42dep", by = "c172code", proportions = "row") |> 
  as.prop.table()
#> Removing NA values from frequency table.
#> [[1]]
#>   low level of education intermediate level of education
#> 1             0.00000000                      1.00000000
#> 2             0.00000000                      1.00000000
#> 3             0.14285714                      0.57142857
#> 4             0.06349206                      0.66666667
#>   high level of education
#> 1              0.00000000
#> 2              0.00000000
#> 3              0.21428571
#> 4              0.15873016

@strengejacke
Copy link
Member Author

Yes, maybe we clarify the documentation so it says "when proportions were calculated" or similar. Should we also move the as.table() docs to the docs from as.prop.table()?

@etiennebacher
Copy link
Member

Done, I'm actually not sure this leads to better docs. Let me know what you think, I can also remove my commits if needed.

@strengejacke
Copy link
Member Author

Thanks, lgtm!

@etiennebacher etiennebacher merged commit 6fc1426 into main Oct 11, 2025
24 of 25 checks passed
@etiennebacher etiennebacher deleted the strengejacke/issue655 branch October 11, 2025 08:21
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.

Extracting proportions from data_tabulate()

3 participants