Skip to content

Conversation

@Nytelife26
Copy link
Contributor

@Nytelife26 Nytelife26 commented Jul 5, 2021

This relates to...

Adding threshold checks. This blocks #802. This is blocked by #1217 and #1218.

Rationale

This is useful for situations such as in
checks.typography.exclamation.30ppm where a PPM threshold metric
determines whether to raise an error or not.

Changes

  • Add tools.threshold_check
  • Convert checks.typography.exclamation to use threshold_check

Features

  • Add tools.threshold_check

Bug Fixes

N/A.

Breaking Changes and Deprecations

N/A.

@Nytelife26 Nytelife26 requested a review from suchow as a code owner July 5, 2021 18:14
@codecov
Copy link

codecov bot commented Jul 5, 2021

Codecov Report

Merging #1188 (f0e34e6) into main (ef6f77d) will increase coverage by 0.03%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1188      +/-   ##
==========================================
+ Coverage   94.40%   94.44%   +0.03%     
==========================================
  Files          83       83              
  Lines        1162     1170       +8     
==========================================
+ Hits         1097     1105       +8     
  Misses         65       65              
Flag Coverage Δ
macos-latest 94.44% <100.00%> (+0.03%) ⬆️
py3.6 93.86% <100.00%> (+0.04%) ⬆️
py3.7 93.86% <100.00%> (+0.04%) ⬆️
py3.8 94.44% <100.00%> (+0.03%) ⬆️
py3.9 94.44% <100.00%> (+0.03%) ⬆️
pypypy3 93.86% <100.00%> (+0.04%) ⬆️
ubuntu-latest 94.44% <100.00%> (+0.03%) ⬆️
windows-latest 94.44% <100.00%> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
proselint/checks/typography/exclamation.py 100.00% <100.00%> (ø)
proselint/tools.py 87.54% <100.00%> (+0.63%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ef6f77d...f0e34e6. Read the comment docs.

suchow
suchow previously requested changes Jul 6, 2021
Copy link
Contributor

@suchow suchow left a comment

Choose a reason for hiding this comment

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

Rather than implementing this as a meta-check, I think we should either:

  1. Create a decorator @threshold that takes a parameter specifying the threshold ppm below above which errors will be flagged.
  2. Do what we are currently doing with max_errors via the truncate_to_max function.

I think I'd put in a vote for making both threshold and max_errors decorators that would work for all check types.

@Nytelife26 Nytelife26 force-pushed the feat/threshold-check branch 2 times, most recently from 1c13437 to a9a68fc Compare August 22, 2021 04:07
@Nytelife26 Nytelife26 requested a review from suchow August 22, 2021 04:10
@Nytelife26 Nytelife26 force-pushed the feat/threshold-check branch from a9a68fc to d869639 Compare August 22, 2021 04:43
@Nytelife26 Nytelife26 changed the title add threshold check convert meta-checks to decorators Aug 22, 2021
@Nytelife26 Nytelife26 force-pushed the feat/threshold-check branch from d869639 to b4e4dda Compare August 22, 2021 20:58
@Nytelife26 Nytelife26 force-pushed the feat/threshold-check branch from b4e4dda to d51501d Compare November 6, 2021 20:08
@Nytelife26 Nytelife26 changed the title convert meta-checks to decorators feat: convert meta-checks to decorators Nov 7, 2021
@Nytelife26 Nytelife26 merged commit 46007a2 into main Nov 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: feat A new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants