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

speeling fixes #5858

Merged
merged 7 commits into from
Feb 19, 2021
Merged

speeling fixes #5858

merged 7 commits into from
Feb 19, 2021

Conversation

levbishop
Copy link
Member

Fixed a bunch of typos.
Added a local wordlist and pylint spelling configuration - makes it a little less noisy, but far from quiet enough to enable by default.

@ajavadia
Copy link
Member

I like that your commit says "speeling fixes" ;)

@woodsp-ibm
Copy link
Member

Nice - we had spelling running on Aqua since the docstrings end up as the published documentation it made sense to us to fix them in build.

I note you changed parametrized to parameterized. I recall looking at that and the former is an accepted spelling but standardizing on the latter seems fine.

In your exception list you have favour and while I can approve of that along with colour, neighbour etc I would have thought it should be favor since its supposed to all be US English, no?

@levbishop
Copy link
Member Author

Nice - we had spelling running on Aqua since the docstrings end up as the published documentation it made sense to us to fix them in build.

Yeah it would be nice to get to the point to turn it on by default, but the pylint spelling checker is fairly unintelligent and for example doesn't add the names that are in scope locally to the wordlist, doesn't ignore sphinx directives, inline tex, etc. Don't want to discourage contributors from writing more docstrings by making them fight with pylint.

I note you changed parametrized to parameterized. I recall looking at that and the former is an accepted spelling but standardizing on the latter seems fine.

Parametrize{,d} looked so obviously wrong to me I didn't even think to check... Seems like both are used, but parameterize is quite a bit more common.

In your exception list you have favour and while I can approve of that along with colour, neighbour etc I would have thought it should be favor since its supposed to all be US English, no?

My schooling coming through there. Probably best to go all US.

@levbishop
Copy link
Member Author

In your exception list you have favour and while I can approve of that along with colour, neighbour etc I would have thought it should be favor since its supposed to all be US English, no?

My schooling coming through there. Probably best to go all US.

On the other hand we already have (favoured internationally) JobStatus.CANCELLED in the code (not just docs) and I don't feel the need break the API and switch to (favored in US) JobStatus.CANCELED etc as long as we are consistent, which we seem to be.

@mergify mergify bot merged commit 1ff533d into Qiskit:master Feb 19, 2021
@levbishop levbishop deleted the pylint-spelling-2 branch February 19, 2021 01:13
@kdk kdk added the Changelog: None Do not include in changelog label Mar 31, 2021
ElePT pushed a commit to ElePT/qiskit that referenced this pull request Jun 27, 2023
* speeling fixes

* lint

* pylint-spelling partial enable

* UK->US

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
ElePT pushed a commit to ElePT/qiskit-algorithms-test that referenced this pull request Jul 17, 2023
* speeling fixes

* lint

* pylint-spelling partial enable

* UK->US

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
ElePT pushed a commit to ElePT/qiskit-algorithms that referenced this pull request Jul 27, 2023
* speeling fixes

* lint

* pylint-spelling partial enable

* UK->US

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: None Do not include in changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants