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

Fix #478 : Rename shift and clock matrices to gen_pauli_x and gen_pauli_z #479

Merged
merged 2 commits into from
Feb 24, 2024

Conversation

MohitKambli
Copy link
Contributor

Description

As requested in #478, I have replaced all the variables/comments/function-names/file-names occurrences from shift to gen_pauli_x and clock to gen_pauli_z.
If at all there are any issues regarding these code changes, then do let me know.
Sincere apologies if I have missed any code to be changed or implemented a code incorrectly.
Kindly let me know if any changes are required.

Todos

Notable points that this PR has either accomplished or will accomplish.

  • Name change of shift/clock to gen_pauli_x/gen_pauli_z.

Questions

  • Question1

Status

  • Ready to go

@purva-thakre
Copy link
Collaborator

purva-thakre commented Feb 23, 2024

@MohitKambli There are going to be some workflow failures based on the changes you introduced. Refer to the Testing and code_style sections in the docs. https://toqito.readthedocs.io/en/latest/contributing.html#testing

The toqito/matrices module has a folder labeled tests with unit tests for the functions in the module. These require changes as well.

Also make sure the functions you change are not used anywhere else: code of other functions or docstring examples of other functions. We have a workflow that checks for both.

@MohitKambli
Copy link
Contributor Author

Hello @purva-thakre,
Hope you're doing well.
Sincere apologies for my mistakes.
I went through the ruff and pylint documentation so that I can test my changes.
I figured out where I was going wrong and made changes accordingly.
Thank you for helping me out and letting me know about these code changes.
I have added a commit where I have made the potential changes.
Hope these changes help in resolving the issues I observed.
Sincere apologies if I messed up somewhere.

Thanks and Regards,
Mohit Kambli

Copy link

codecov bot commented Feb 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.1%. Comparing base (2c9f0b6) to head (cd6d4d2).
Report is 9 commits behind head on master.

Additional details and impacted files
@@          Coverage Diff           @@
##           master    #479   +/-   ##
======================================
  Coverage    98.1%   98.1%           
======================================
  Files         161     161           
  Lines        3096    3096           
  Branches      753     753           
======================================
  Hits         3038    3038           
  Misses         37      37           
  Partials       21      21           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@vprusso
Copy link
Owner

vprusso commented Feb 24, 2024

@MohitKambli There are going to be some workflow failures based on the changes you introduced. Refer to the Testing and code_style sections in the docs. https://toqito.readthedocs.io/en/latest/contributing.html#testing

The toqito/matrices module has a folder labeled tests with unit tests for the functions in the module. These require changes as well.

Also make sure the functions you change are not used anywhere else: code of other functions or docstring examples of other functions. We have a workflow that checks for both.

Hi @MohitKambli. Looks good overall, but I believe you may need to address the comment here that @purva-thakre mentioned. Let us know if that doesn't make sense, and thanks again for your contribution! I think it's close!

@purva-thakre
Copy link
Collaborator

@vprusso I think this PR is good to go. The part with the tests folder won't apply here given the conversation in #485

@vprusso
Copy link
Owner

vprusso commented Feb 24, 2024

Awesome, also good on my end as well. Merge when ready, @MohitKambli (after you've addressed the docs issue as mentioned by @purva-thakre )

@MohitKambli
Copy link
Contributor Author

Hello @vprusso and @purva-thakre,
Hope you're doing well.
As mentioned by @purva-thakre in the previous comment, I had made the code changes and executed the ruff and pylint commands for coding style and testing purposes respectively.
If there are any changes to make in the documentation or anywhere else, then do let me know.
I didn't quite get what needs to be done from my end.
Sincere apologies if I misunderstood anything or missed some part.

Thanks and Regards,
Mohit Kambli

@purva-thakre purva-thakre merged commit f613473 into vprusso:master Feb 24, 2024
18 checks passed
@purva-thakre
Copy link
Collaborator

purva-thakre commented Feb 24, 2024

If there are any changes to make in the documentation or anywhere else, then do let me know.

@MohitKambli I think @vprusso's last comment is through his email. he might not have noticed that all workflow jobs pass.

Thanks!

@purva-thakre purva-thakre added this to the v1.0.8 milestone Mar 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants