-
Notifications
You must be signed in to change notification settings - Fork 2.7k
chore: update comments to fix translation problems in documentation #15350
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
base: main
Are you sure you want to change the base?
Conversation
…`Pauli`, `PauliLocation` and `BitTerm` objects
|
Thank you for opening a new pull request. Before your PR can be merged it will first need to pass continuous integration tests and be reviewed. Sometimes the review process can be slow, so please be patient. While you're waiting, please feel free to review other open PRs. While only a subset of people are authorized to approve pull requests for merging, everyone is encouraged to review open pull requests. Doing reviews helps reduce the burden on the core team and helps make the project's code better for everyone. One or more of the following people are relevant to this code:
|
|
|
Eric-Arellano
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @Osalotioman! Qiskit SDK team: this change is to fix a problem in documentation with translations.
@Osalotioman, after seeing this in your PR, I realize that the docs will read better by using 'instances' rather than 'objects'. Would you be willing to please update that?
|
Sure. |
…or class pluralization
|
I found other places where “objects” was used and updated them to use “instances” accordingly. |
Eric-Arellano
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! 🚀 We'll need a Qiskit SDK core maintainer to review this, but looks great to me
jakelishman
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy in general to merge a PR on these lines, but I disagree with the automated renaming of every "object" into "instance" - 2/3 languages in Qiskit are not OO languages, and the term "instance" sounds wrong to me in the C and Rust context. I can accept it in Python, but I don't even really agree that it reads better in all cases there. The use of the neutral "object" is deliberate - "instance" has a specific meaning that doesn't always apply.
I'd also generally prefer we don't churn code comments that are not built into public-facing documentation, but at this point I'm not asking to revert those.
Please can we at least revert changes on those lines in the Rust and C-API files? The changes related to replacing ``Class``s are all ok.
|
I have updated the rust code comments along with readme files for directories associated with rust code to use the neutral I also restored the release note files, since they have already been compiled with the releases and changing them will lead to inconsistencies. |
Eric-Arellano
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @Osalotioman! Pardon my bad advice earlier to use instances rather than objects. Thank you for updating that back.
To clarify Jake's request, he is asking that you please revert back any time in Python that you switched the word object to instance to go back to using object. Ideally, the diff will only be changing when we use 's and shouldn't change anything else.
chore: Use objects in docs instead of instances
|
Thankyou @Eric-Arellano for clarifying, I had initially thought it was just to revert it in the rust files. |
Eric-Arellano
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! Looks great. I appreciate you iterating on this with the reviews.
We'll need a Qiskit SDK maintainer like @jakelishman to merge
Summary
Updated text, comments and docstrings across multiple Markdown, Rust and Python files to update references to objects.
This addresses part of issue2067 from the Qiskit documentation repo.
Details and comments
In the documentation, multiple instances of a class were sometimes referred to using the style "
MyClasss".This PR updates these references to "
MyClassinstances" to fix translation problems in documentation.