Repository navigation
Add codespell, consistent US english style & correct typos - #706
purva-thakre wants to merge 29 commits into
Conversation
0c0dec4 to
2bbe4a4
Compare
|
This PR is ready for a review. Note that the PR only makes non-breaking changes. There are spelling related public API changes that we should discuss before adding the respective PRs. Codespell flags them, but they are preserved in this branch (via 1.
|
0e284ba to
326000a
Compare
| "RUNNING": JobStatus.RUNNING, | ||
| "DONE": JobStatus.DONE, | ||
| "CANCELLED": JobStatus.CANCELLED, | ||
| "CANCELED": JobStatus.CANCELLED, |
There was a problem hiding this comment.
This is a behavior change, we should not change this without veryfying that it still works
There was a problem hiding this comment.
Do you mean on actual hardware?
| "RUNNING": JobStatus.RUNNING, | ||
| "DONE": JobStatus.DONE, | ||
| "CANCELLED": JobStatus.CANCELLED, | ||
| "CANCELED": JobStatus.CANCELLED, |
There was a problem hiding this comment.
Another behavior change
positr0nium
left a comment
There was a problem hiding this comment.
Thanks for pushing this :) Some minor changes with API keywords in some backends.
497c17b to
84ac927
Compare
bastibock
left a comment
There was a problem hiding this comment.
Thank you for taking care of this Purva!
There were obviously quite some spelling mistakes in the codebase.
I flagged some more behavior changes that should be done automatically.
And did you check, that the documentation compiles correctly? Especially the two figures, that were marked as changed?
And for the future this would mean, that in case someone adds some function or parameter, they really have to be aware of the spelling. So if they want to intentionally spell something differently they have to exclude it manually, corrrect?
| References | ||
| ---------- | ||
| - Childs et al. (2017) `Quantum algorithm for systems of linear equations with exponentially improved dependence on precision <https://arxiv.org/pdf/1511.02306>`_. | ||
| - Gilyén et al. (2019) `Quantum singular value transformation and beyond: exponential improvements for quantum matrix arithmetics <https://dl.acm.org/doi/abs/10.1145/3313276.3316366>`_. |
There was a problem hiding this comment.
I would keep 'arithmetics' here, since this is the original title.
| References | ||
| ---------- | ||
| - Childs et al. (2017) `Quantum algorithm for systems of linear equations with exponentially improved dependence on precision <https://arxiv.org/pdf/1511.02306>`_. | ||
| - Gilyén et al. (2019) `Quantum singular value transformation and beyond: exponential improvements for quantum matrix arithmetics <https://dl.acm.org/doi/abs/10.1145/3313276.3316366>`_. |
There was a problem hiding this comment.
Same as above. Stick with 'arithmetics'
| _DIAGONAL_1Q = frozenset({"p", "rz", "z", "s", "t", "s_dg", "t_dg", "id", "gphase"}) | ||
|
|
||
|
|
||
| def _is_cancelled_by_zero(op: Operation, qubits: list[Qubit], fresh: set[Qubit]) -> bool: |
There was a problem hiding this comment.
This also looks like an API change. But probably no problem to change it here, since it's only an internal method.
There was a problem hiding this comment.
Yes, I figured since it's a private method, we don't need to worry about it.
| "completed": JobStatus.DONE, | ||
| "failed": JobStatus.ERROR, | ||
| "cancelled": JobStatus.CANCELLED, | ||
| "canceled": JobStatus.CANCELLED, |
There was a problem hiding this comment.
Looks like another behaviour change. Or is this conform with iqm structure?
There was a problem hiding this comment.
Nope, same as the others, this is a behavior change.
| aqt_job = _make_aqt_job(fail=RuntimeError("canceled")) | ||
| aqt_job.status.return_value = MagicMock() | ||
| aqt_job.status.return_value.name = "CANCELLED" | ||
| aqt_job.status.return_value.name = "CANCELED" |
There was a problem hiding this comment.
More behaviour changes.
If the changes flagged from Raphael can be done, these are correct. Otherwise the tests have to stay the same as well for now.
There was a problem hiding this comment.
Well, that's the thing, with the changes flagged by @positr0nium , I figured they were good because the unit tests passed after I made the changes. But unless there is some behavior that is not covered by unit tests, I could make these changes later down the line.
| qiskit_job = _make_qiskit_job(fail=RuntimeError("canceled")) | ||
| qiskit_job.status.return_value = MagicMock() | ||
| qiskit_job.status.return_value.name = "CANCELLED" | ||
| qiskit_job.status.return_value.name = "CANCELED" |
There was a problem hiding this comment.
More behaviour changes in the qiskit-backend test.
Yes, there is a check in the CI similar to Qrisp/.github/workflows/lint.yml Lines 40 to 47 in 5f26244 I intentionally misspelled
|
Good point. I did not check this initially 😅 .
|
fe3080d to
a549dfa
Compare
|
The latest commits should address all the reviewer comments. As suggested by both, I went ahead and undid the behavior changes. I guess these are better handled with #706 (comment).
I ask this because there are a lot more incorrectly named functions. They were not flagged by FindingsThe Context column shows where the misspelling lives:
1.
|
| Identifier | Context | Role | Files |
|---|---|---|---|
ammend_inpl_adder |
identifier | function | adder_tools.py |
ammend_cl_int |
identifier | variable | adder_tools.py |
add_ammend_anc |
string | — | adder_tools.py |
2. ammended → amended
| Identifier | Context | Role | Files |
|---|---|---|---|
ammended_func |
identifier | function | custom_control_environment.py, custom_inversion_environment.py |
ammended_function |
identifier | function | centerclass.py, qaching.py |
ammended_adder |
identifier | function | adder_tools.py |
ammended_args |
identifier | variable | custom_control_environment.py, boolean_simulation.py, qc_extraction_interpreter.py, centerclass.py |
ammended_kwargs |
identifier | variable | centerclass.py, qaching.py |
3. signficance → significance
| Identifier | Context | Role | Files |
|---|---|---|---|
signficance_range_intersetion |
identifier | variable | adder_tools.py, ripple_carry_adder.py |
4. dirtyness → dirtiness
| Identifier | Context | Role | Files |
|---|---|---|---|
permit_dirtyness |
identifier | variable | adder_tools.py, backtracking_tree.py, from_projector.py, quantum_variable.py, quantum_float.py |
5. multiplyer / multiplyers → multiplier / multipliers
| Identifier | Context | Role | Files |
|---|---|---|---|
jasp_controlling_multiplyer |
identifier | function | jasp_multiplyers.py |
jasp_controlling_multiplyer |
identifier | variable | __init__.py (jasp_arithmetic) |
jasp_multiplyer |
identifier | function | jasp_multiplyers.py |
jasp_multiplyer |
identifier | variable | __init__.py (jasp_arithmetic), quantum_float.py |
jasp_multiplyers |
filename | — | jasp_multiplyers.py itself |
6. spacial → spatial
| Identifier | Context | Role | Files |
|---|---|---|---|
spacial_to_spin |
identifier | function | electronic_structure.py |
num_spacial_orbs |
identifier | variable | electronic_structure.py |
7. neighbour → neighbor
| Identifier | Context | Role | Files |
|---|---|---|---|
_fuse_swap_with_neighbour |
identifier | function | fuse_adjacents.py |
8. inital → initial
| Identifier | Context | Role | Files |
|---|---|---|---|
inital_qvs |
identifier | variable | iteration_environment.py |
9. burnin → burning / burn in
| Identifier | Context | Role | Files |
|---|---|---|---|
num_burnin |
string | — | centerclass.py |
10. cancelation → cancellation
| Identifier | Context | Role | Files |
|---|---|---|---|
cancelation_bool |
string | — | rus.py |
11. identifer → identifier
| Identifier | Context | Role | Files |
|---|---|---|---|
alloc_identifer |
identifier | variable | memory_management.py |
12. prefered → preferred
| Identifier | Context | Role | Files |
|---|---|---|---|
prefered_nodes |
identifier | variable | memory_management.py |
13. orignal → original
| Identifier | Context | Role | Files |
|---|---|---|---|
orignal_shape_other |
identifier | variable | bi_arrays.py |
Summary: 13 distinct misspellings, 11 in real Python identifiers + 2 in strings/filenames, across 15 files.
Of the 13, codespell's default config ([\w\-'’]+ tokenizer) would still flag some occurrences if the misspelled word appears as a standalone token in a comment or string. The blind spot is specifically when the misspelling is glued to another word via _, making it a single token. For example, ammend and burnin alone would each be caught, but ammend_inpl_adder and num_burnin are not.
|
I would say we use option 1 for public facing (i.e. shown in the documentation) methods. But, we can directly rename internal methods or public examples. For example:
|
|
going to start over with a new PR. There are way too many merge conflicts. |

Description
https://github.com/codespell-project/codespell
This PR integrates
codespellinto the CI pipeline and fixes non-API British spellings found across the codebase. Public API names that use British spelling (e.g.,Job.cancelled(),Jaspr.embedd()) are preserved via the ignore list or inline# codespell:ignoremarkers. The latter will be handled by separate PRs.Configuration Changes
.github/workflows/lint.yml(renamed fromruff_checks.yml)Codespellstep to CI (runs on every PR/push tomain)codespellwith--builtin clear,rare,en-GB_to_en-USdictionariesRuff formatcheckcodespelllocallypyproject.toml[tool.codespell]section with:builtin = "clear,rare,en-GB_to_en-US"— unambiguous errors, rare words, British→Americancheck-filenames = true— also checks file namescount = trueandsummary = true.venv,.git, virtual envs, caches, binary files, generated doc build, pre-0.10 changelogsignore-words-list— false positives (ancilla, ket, Aer, MIS, proper names, etc.)codespell<=2.4.2todev-code-styleoptional dependenciesRelated Issues
Closes #
Related to #
Type of Change
Breaking Change?
If yes, describe the impact and migration path:
What was changed?
How was it tested?
Screenshots / Output (if applicable)
Checklist
Reviewer Notes