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 #7438 - Remove redundant skip in test #7519

Closed
wants to merge 5 commits into from

Conversation

TheGupta2012
Copy link
Contributor

@TheGupta2012 TheGupta2012 commented Jan 11, 2022

Summary

Fixes #7438 by removing redundant

Details and comments

This test was disabled wrongly, given that Pull Request 741 in aer was merged and newer versions of aer have been released since.

@TheGupta2012 TheGupta2012 requested a review from a team as a code owner January 11, 2022 16:51
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.

You're welcome to go through this and sort it out, but I think it might be a little more complex than just removing the skip. I had a very brief go before Christmas, and I think I hit some old code in the ConfigurableFakeBackend that was pretty out-of-date and might need a fair bit of updating. I don't remember 100%, though.

@TheGupta2012 TheGupta2012 marked this pull request as draft January 12, 2022 16:10
@TheGupta2012
Copy link
Contributor Author

You're welcome to go through this and sort it out, but I think it might be a little more complex than just removing the skip. I had a very brief go before Christmas, and I think I hit some old code in the ConfigurableFakeBackend that was pretty out-of-date and might need a fair bit of updating. I don't remember 100%, though.

Okay, I shall look into it. Thanks for the update!

@TheGupta2012
Copy link
Contributor Author

  • @jakelishman, would you be able to provide an insight to the most recent test fail as I wasn't really familiar with qiskit pulse.
  • Besides that, ConfigurableBackend had a parameter version which stood for backend_version. This was overlapping with the version parameter in the BackendV2 and thus was changed to fix previous traceback.

@HuangJunye HuangJunye added the Community PR PRs from contributors that are not 'members' of the Qiskit repo label Jun 21, 2022
@jakelishman
Copy link
Member

At @javabster's request, I've fixed the merge conflicts that were in this PR in order to have it run its CI again, but I don't know any context. My only comment above was just that I'd attempted to remove the unittest.skip line before and seen some failures (and sorry that I never replied to the question at the time).

It's possible that #9168 could be used instead of this PR, but I don't know the context of either PR, nor of the code in question here.

@1ucian0
Copy link
Member

1ucian0 commented Apr 25, 2023

Closing in favor #9168 . Thanks!

@1ucian0 1ucian0 closed this Apr 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Community PR PRs from contributors that are not 'members' of the Qiskit repo
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Enable ignored test in test_fake_backends
4 participants