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 BasicSwap FakeRun Typo #10149

Closed
wants to merge 0 commits into from
Closed

Fix BasicSwap FakeRun Typo #10149

wants to merge 0 commits into from

Conversation

evmckinney9
Copy link
Contributor

@evmckinney9 evmckinney9 commented May 23, 2023

Summary

Resolve #10147

Details and comments

Reference the function instead of the attribute.

@qiskit-bot qiskit-bot added the Community PR PRs from contributors that are not 'members' of the Qiskit repo label May 23, 2023
@evmckinney9
Copy link
Contributor Author

I checked other files, didn't see a similar mistake.

@evmckinney9 evmckinney9 marked this pull request as ready for review May 23, 2023 23:00
@evmckinney9 evmckinney9 requested a review from a team as a code owner May 23, 2023 23:00
@qiskit-bot
Copy link
Collaborator

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 the following people are requested to review this:

  • @Qiskit/terra-core

@mtreinish mtreinish added stable backport potential The bug might be minimal and/or import enough to be port to stable Changelog: Bugfix Include in the "Fixed" section of the changelog labels May 23, 2023
@Cryoris
Copy link
Contributor

Cryoris commented May 24, 2023

Thanks for the fix! Could you add a test that verifies this behavior is fixed now (in the basic swap test file) and a releasenote documenting this fix? See e.g. the contributing guidelines for some more details and let us know if we can help you with that!

@1ucian0
Copy link
Member

1ucian0 commented May 24, 2023

Thanks for the fix! Do you mind adding a test to show the bug that this is fixing?

@coveralls
Copy link

coveralls commented May 24, 2023

Pull Request Test Coverage Report for Build 5081968106

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • 50 unchanged lines in 9 files lost coverage.
  • Overall coverage increased (+0.05%) to 85.968%

Files with Coverage Reduction New Missed Lines %
qiskit/transpiler/layout.py 1 94.08%
crates/accelerate/src/vf2_layout.rs 3 94.74%
qiskit/transpiler/passes/calibration/rzx_builder.py 3 89.53%
crates/accelerate/src/sabre_swap/layer.rs 4 97.32%
qiskit/circuit/library/generalized_gates/rv.py 4 85.19%
crates/qasm2/src/lex.rs 5 90.63%
qiskit/converters/init.py 5 73.08%
crates/qasm2/src/parse.rs 6 97.58%
qiskit/compiler/transpiler.py 19 90.12%
Totals Coverage Status
Change from base Build 5059475398: 0.05%
Covered Lines: 71356
Relevant Lines: 83003

💛 - Coveralls

evmckinney9 added a commit to evmckinney9/qiskit-evmckinney9 that referenced this pull request May 24, 2023
Copy link
Member

@jakelishman jakelishman left a comment

Choose a reason for hiding this comment

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

Thanks for looking at this! I have one thing about the test, but after that we'll be good to merge.

test/python/transpiler/test_basic_swap.py Outdated Show resolved Hide resolved
@evmckinney9
Copy link
Contributor Author

evmckinney9 commented Jun 14, 2023

Oops, seems I messed things up by moving around my fork's branches.

@jakelishman
Copy link
Member

No worries, it's not a problem. In case you weren't sure, this is why it's not generally recommended to make PRs from your main branch - GitHub will let you because of its fork+PR process, but it's pretty easy to get the branches tangled.

jakelishman pushed a commit that referenced this pull request Jun 14, 2023
* fix typo, issue #10147

* create basicswap fake_run test

* release note for #10149 fix

* ensure fake_run modifes layout prop, not the circuit logic

* black formatting, test_basic_swap
mergify bot pushed a commit that referenced this pull request Jun 14, 2023
* fix typo, issue #10147

* create basicswap fake_run test

* release note for #10149 fix

* ensure fake_run modifes layout prop, not the circuit logic

* black formatting, test_basic_swap

(cherry picked from commit 7df290b)
jakelishman pushed a commit that referenced this pull request Jun 14, 2023
* fix typo, issue #10147

* create basicswap fake_run test

* release note for #10149 fix

* ensure fake_run modifes layout prop, not the circuit logic

* black formatting, test_basic_swap

(cherry picked from commit 7df290b)

Co-authored-by: Evan McKinney <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: Bugfix Include in the "Fixed" section of the changelog Community PR PRs from contributors that are not 'members' of the Qiskit repo stable backport potential The bug might be minimal and/or import enough to be port to stable
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

BasicSwap FakeRun Typo
7 participants