Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simple regex checker for passive voice constructions #802

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

RyanMcCarl
Copy link

This is my first attempt to contribute, so please let me know if I need to do anything differently -- e.g. register the check with the application.

I haven't tested the regex extensively, but it performed well on this list.

Here are the results on Pythex:

Passive voice regex test 2018-06-12.pdf

Here is the regex itself -- I'm sure it can be improved, modularized into several regexes, etc. Happy to continue working on it when I can if you like the idea.

(\b(?:be|am|is|are|was|were|have|has|had)\b[\w\s]{,15}?(?:d|(?<!whe)n|ne|left|being)\b(?: by\b)?)

@RyanMcCarl
Copy link
Author

I'm having trouble getting this through the checks even after rebasing the repository. Can anyone tell me what I'm doing wrong? @mpacer @suchow

@Nytelife26
Copy link
Member

It seems that for the most part this PR is, well, broken. Its contents appear to be lost in over 372 changed files. Using regexes to hack together a passive voice check also doesn't seem ideal.

With that being said, I think we'll have to close this, unless you're available to work on it @RyanMcCarl.

@Nytelife26 Nytelife26 added cat: new-rule Issues and PRs related to new proselint rules. priority: null Issues and PRs that are of negligible importance so may be postponed. status: postponed Issues and PRs that are being temporarily set aside in favour of others. type: feat Issues and PRs related to new features. version: minor Issues and PRs with new features belonging to the next minor release. labels May 24, 2021
@suchow
Copy link
Member

suchow commented May 24, 2021

@Nytelife26 Even if Ryan can’t work on this, it’s worth salvaging what we can from the PR. There are new modules buried in here based on Ryan’s professional expertise (he’s a lawyer and scholar who has taught advanced legal writing courses and does research on use of A.I. in law).

@suchow
Copy link
Member

suchow commented May 24, 2021

In particular, see proselint/checks/mccarl/rm_style_pref_forms.py

@Nytelife26
Copy link
Member

Ah, I see. I may have to root through the diffs and cherry-pick useful files, then - I didn't have time to review them all due to the mostly broken structure and the number of file changes.

I'll take a look now, though.

@Nytelife26 Nytelife26 force-pushed the master branch 2 times, most recently from 18fc718 to 3344df8 Compare May 25, 2021 20:44
@Nytelife26 Nytelife26 added status: wip Issues and PRs that are still a work in progress. and removed status: postponed Issues and PRs that are being temporarily set aside in favour of others. labels May 25, 2021
@amperser amperser deleted a comment from codecov bot May 27, 2021
@amperser amperser deleted a comment from ProselintBot May 27, 2021
@amperser amperser deleted a comment from ProselintBot May 27, 2021
@Nytelife26 Nytelife26 added status: blocked Issues and PRs that cannot be resolved until others are. and removed status: wip Issues and PRs that are still a work in progress. labels May 28, 2021
@codecov
Copy link

codecov bot commented May 30, 2021

Codecov Report

Merging #802 (7ada6b8) into main (f8928ad) will increase coverage by 0.03%.
The diff coverage is 100.00%.

❗ Current head 7ada6b8 differs from pull request most recent head b48ea45. Consider uploading reports for the commit b48ea45 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##             main     #802      +/-   ##
==========================================
+ Coverage   94.72%   94.75%   +0.03%     
==========================================
  Files          83       84       +1     
  Lines        1213     1221       +8     
==========================================
+ Hits         1149     1157       +8     
  Misses         64       64              
Flag Coverage Δ
macos-latest 94.75% <100.00%> (+0.03%) ⬆️
py3.6 94.24% <100.00%> (+0.03%) ⬆️
py3.7 94.24% <100.00%> (+0.03%) ⬆️
py3.8 94.75% <100.00%> (+0.03%) ⬆️
py3.9 94.75% <100.00%> (+0.03%) ⬆️
pypypy3 94.24% <100.00%> (+0.03%) ⬆️
ubuntu-latest 94.75% <100.00%> (+0.03%) ⬆️
windows-latest 94.75% <100.00%> (+0.03%) ⬆️

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

Impacted Files Coverage Δ
proselint/checks/passive_voice/misc.py 100.00% <100.00%> (ø)

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 f8928ad...b48ea45. Read the comment docs.

@Nytelife26
Copy link
Member

I will review the Garner checks now as it is likely they are already present in another check since we have most of GMEU implemented. The McCarl checks and passive voice check will additionally need to be split up into two separate PRs before this can continue.

@RyanMcCarl RyanMcCarl requested a review from suchow as a code owner July 5, 2021 14:32
@Nytelife26 Nytelife26 added status: wip Issues and PRs that are still a work in progress. and removed status: blocked Issues and PRs that cannot be resolved until others are. labels Jul 5, 2021
@Nytelife26 Nytelife26 force-pushed the master branch 3 times, most recently from 71139a1 to 820870b Compare July 5, 2021 15:20
@Nytelife26
Copy link
Member

Nytelife26 commented Jul 5, 2021

The regex needs to be adjusted to pass the special cases checks. Perhaps the exceptions functionality might make that easier.

@Nytelife26 Nytelife26 added status: review-ready PRs that are ready for author review. and removed status: wip Issues and PRs that are still a work in progress. labels Jul 5, 2021
@Nytelife26 Nytelife26 added status: blocked Issues and PRs that cannot be resolved until others are. and removed status: review-ready PRs that are ready for author review. labels Jul 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cat: new-rule Issues and PRs related to new proselint rules. priority: null Issues and PRs that are of negligible importance so may be postponed. status: blocked Issues and PRs that cannot be resolved until others are. type: feat Issues and PRs related to new features. version: minor Issues and PRs with new features belonging to the next minor release.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants