Open
Conversation
Introduces a 'mitigation' field on ACRO / Tables that can be 'none', 'suppress', or 'round'. 'round' applies whole-table rounding to the nearest 'round_base' (default 5, configurable via 'safe_round_base' in the yaml). The old boolean 'suppress' is preserved as a backward- compatible property. 'margins=True' is rejected under rounding because rounded totals would not add up and are a known attack vector. Implements issue #126.
ssrhaso
requested review from
Copilot and
jim-smith
and removed request for
Copilot
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #381 +/- ##
==========================================
+ Coverage 99.70% 99.72% +0.02%
==========================================
Files 9 9
Lines 1354 1457 +103
==========================================
+ Hits 1350 1453 +103
Misses 4 4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Adds unit tests for mitigation/round_base options on acro init and for the enable_rounding/disable_rounding session commands, lifting stata parser patch coverage from 60% to 99%.
- Adds D213 summary-on-second-line and D413 trailing-blank-line to the four new docstrings Codacy flagged. - Adds the missing Tables.__init__ docstring. - Drops the unused round_base kwarg from Tables.__init__ (ACRO sets round_base via the property setter after reading yaml, so the kwarg path was dead in practice). - Adds a unit test for the missing-values branch of _rounded_summary to close the codecov patch gap.
Codacy enables D212 and D213 simultaneously (they are mutually exclusive), and pydocstringformatter strips the trailing blank line D413 requires. Single-line docstrings sidestep all three. Type hints still document the parameters.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a
mitigationfield on ACRO/Tables:"none","suppress", or"round"."round"rounds every cell ofcrosstab/pivot_tableoutput to the nearestround_base(default 5, set viasafe_round_basein the yaml).The old
suppressbool,enable_suppression/disable_suppression, and the Statasuppressoption still work as aliases, so existing scripts don't break. New helpersenable_rounding(base=...)/disable_rounding()mirror the suppression pair.margins=Trueis rejected under rounding - rounded inner cells don't add up to rounded totals and that's a known attack vector (per the bounds-tightening discussion on #126). SDC audit records keep the raw disclosure-check results so rounding can't mask threshold failures.If needed we can switch
margins=Truefrom hard-fail to warn-and-strip if thats better user experience. LeftStrEnumfor a follow up!Closes #126.