Adding Reverse Existence checks Issue #1334 #1351
Conversation
Nytelife26
added
type: feat
|
Thank you for contributing! I am postponing this PR until my queries on the related issue are answered, but your work is appreciated nonetheless. In the meantime, I will proceed to review this. |
| msg = "'{}' is not a word kids learn in elementary school." | ||
|
|
||
| elementary = [ | ||
| "a", |
There was a problem hiding this comment.
These files are quite large. I would prefer if we implemented some way to condense them, like having a compressed dictionary available in the folder to import here, to avoid code bloat.
| @memoize | ||
| def check(text): | ||
| """Check the text.""" | ||
| err = "reverse existence.top1000" |
There was a problem hiding this comment.
| err = "reverse existence.top1000" | |
| err = "reverse_existence.elementary" |
This conforms to the directory path, and fixes a naming conflict with top1000.
| def check(text): | ||
| """Check the text.""" | ||
| err = "reverse existence.top1000" | ||
| msg = "'{}' is not a word kids learn in elementary school." |
There was a problem hiding this comment.
To me, this check is not generally applicable enough to be included by default. How often do people write with the intention of exclusively using words understandable to infants? This seems to be more about intention than prose.
My official recommendation so far would be to remove these checks and keep this PR focused on implementing the non-existence mechanic (or exclusivity, more accurately), but I am always open to hearing your thoughts.
| def check(text): | ||
| """Check the text.""" | ||
| err = "reverse existence.top1000" | ||
| msg = "'{}' is not it top 1,000 most popular words." |
There was a problem hiding this comment.
My thoughts here are the same with the elementary check.
| "reverse_existence.top1000": False, | ||
| "reverse_existence.elementary": False |
There was a problem hiding this comment.
Depending on the direction we take with my prior comments, these may need to be adjusted. Disregard this for now.
| @@ -365,6 +365,26 @@ def existence_check(text, list, err, msg, ignore_case=True, str=False, | |||
| return errors | |||
|
|
|||
|
|
|||
| def reverse_existence_check(text, list, err, msg): | |||
There was a problem hiding this comment.
This should support the same options as existence_check, since it calls it internally anyway.
| # containing the prohibited words | ||
| if(len(words_not_in_list) == 0): | ||
| return [] | ||
| return existence_check(text, words_not_in_list, err, msg) |
There was a problem hiding this comment.
It is highly inefficient to locate words that should not be in the source, and then have existence_check search for them anyway. I think it would be better to model this from existence_check, instead of calling it.
| json = "tests/test_config_flag_proselintrc.json" | ||
| overrides = load_options(json, default) |
There was a problem hiding this comment.
The changes to this file appear to be out of scope for this PR. They are unrelated to the functionality of the "reverse existence check," and should therefore be moved into their own PR. Thank you for tidying up, though. I appreciate going beyond the task at hand, and I understand how difficult it can be not to slide small fixes into larger PRs as a bundle.
|
|
||
| comm = "--dump-config --config tests/test_config_flag_proselintrc.json" | ||
| output = runner.invoke( | ||
| proselint, "--dump-config --config tests/test_config_flag_proselintrc.json") | ||
| proselint, comm) |
There was a problem hiding this comment.
Just OOC, why couldn't this have just become this?
output = runner.invoke(
proselint,
"--dump-config --config tests/test_config_flag_proselintrc.json")That's under 80 chars on each line.
(For that matter, it could've been this, too, if it really came down to it. Python supports C-style automatic string-literal concatenation.)
output = runner.invoke(
proselint,
"--dump-config "
"--config tests/test_config_flag_proselintrc.json")
|
closing this for inactivity and in favour of #1370. |
I added a way to ensure that you only words within a certain set as requested in Issue #1334 or as the reporter coin as a "reverse-existence check". Currently, the two sets implemented with this new feature are the top 1000 words set as requested by the initial reporter, but I also added a set of words that elementary kids are taught to learn how to spell. With both sets, I have provided a suite of test cases for quality assurance. However, I did have to modify test_config_flag.py a bit to pass linter tests (some lines were too long, and had to turn a string into a variable on the line above).