Skip to content

BUG: Correct functionality of numpydoc SS05 #613

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

Merged
merged 5 commits into from
Apr 15, 2025

Conversation

maxinelasp
Copy link
Contributor

Currently SS05 checks for infinitive verbs by checking if the last character is "s". However, if the first word is "process", which is valid, this incorrectly throws a numpydoc validation error.

This commit allows the first word to end in two "s"es without flagging as an error. This will allow for words like "process" or "progress," while still checking for invalid words like "creates."

I wasn't sure if I should open an issue for this too, I thought it would be easiest to just find the error and submit a PR, but I'm happy to open an issue as well.

@stefanv
Copy link
Contributor

stefanv commented Mar 11, 2025

Hah, so iffy what we're doing here, so I think your suggestion is fine. Given how non-robust this test is, we should probably add a comment explaining our lightweight checker mechanism.

pre-commit is complaining; happy to run that for you if you have issues running it locally (pi p install pre-commit; pre-commit install once, and you should be good to go.)

@maxinelasp
Copy link
Contributor Author

Apologies, I didn't see that there was a pre-commit in the project! Looks like it's passing now.

@stefanv
Copy link
Contributor

stefanv commented Mar 12, 2025

Great, thanks, can you add a comment to explain what that clause does? Can be as simple as "# Heuristic to check for infinitive verbs". Then I'll go ahead and merge.

Currently SS05 checks for infinitive verbs by checking if the last character is "s". However, if the first word is "process", which is valid, this incorrectly throws a numpydoc validation error.

This commit allows the first word to end in two "s"es without flagging as an error. This will allow for words like "process" or "progress," while still checking for invalid words like "creates."
@maxinelasp
Copy link
Contributor Author

Sure thing, sorry, I wasn't sure if you meant a comment in the code or elsewhere!

Copy link
Contributor

@stefanv stefanv left a comment

Choose a reason for hiding this comment

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

@larsoner Sanity check?

@stefanv
Copy link
Contributor

stefanv commented Mar 12, 2025

Sure thing, sorry, I wasn't sure if you meant a comment in the code or elsewhere!

Thank you very much. numpydoc can do with a lot of love, so we appreciate your contribution.

@maxinelasp
Copy link
Contributor Author

Any updates on this? Looks like it wasn't merged.

Copy link
Collaborator

@larsoner larsoner left a comment

Choose a reason for hiding this comment

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

Maybe someday we need a

@larsoner
Copy link
Collaborator

Ignore that comment... marking for merge-when-green, thanks in advance @maxinelasp and sorry I missed the ping for review!

@larsoner
Copy link
Collaborator

FYI I ended up needing to tweak the tests. Nothing too controversial I think -- removed some no-longer-needed ignores, and changed the Process ... that was a false alarm before to a Creates ... that is a proper hit, and adapted the ignore lines in the tests to ignore it where we ignored Process ... before.

@larsoner larsoner merged commit 5b437b5 into numpy:main Apr 15, 2025
30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants