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

Update options to be a mapping instead of a simple namespace #6355

Closed
wants to merge 3 commits into from

Conversation

mtreinish
Copy link
Member

@mtreinish mtreinish commented May 4, 2021

Summary

This commit changes the options class to be a mapping instead of a
simple namespace. While we wanted to preserve attribute based access
primarily the simple namespace is too limited in practice and involves a
lot of hacking around the internals such as frequent calls to
Options.__dict__ to get an iterator of fields in the options. To improve
the usability of the class without breaking existing users this changes
the base class to be a MutableMapping and adds the necessary methods to
make it usable like a dict or a simple namespace.

Details and comments

@mtreinish mtreinish added on hold Can not fix yet Changelog: API Change Include in the "Changed" section of the changelog labels May 4, 2021
@mtreinish mtreinish added this to the 0.18 milestone May 4, 2021
@mtreinish mtreinish requested review from chriseclectic, jyu00 and a team as code owners May 4, 2021 21:19
This commit changes the options class to be a mapping instead of a
simple namespace. While we wanted to preserve attribute based access
primarily the simple namespace is too limited in practice and involves a
lot of hacking around the internals such as frequent calls to
Options.__dict__ to get an iterator of fields in the options. To improve
the usability of the class without breaking existing users this changes
the base class to be a MutableMapping and adds the necessary methods to
make it usable like a dict or a simple namespace.
@mtreinish mtreinish changed the title [WIP] Update options to be a mapping instead of a simple namespace Update options to be a mapping instead of a simple namespace May 7, 2021
@mtreinish mtreinish removed the on hold Can not fix yet label May 7, 2021
@mtreinish mtreinish added the on hold Can not fix yet label May 7, 2021
@mtreinish
Copy link
Member Author

Ugh, it looks like this is breaking Aer in some way and because of how the QuantumInstance runs circuits things get stuck in an infinite loop on failure causing the algorithms tests to timeout. I'll have to run this locally to debug what I messed up

@kdk kdk modified the milestones: 0.18, 0.19 Jun 10, 2021
@kdk kdk removed this from the 0.19 milestone Oct 27, 2021
@mtreinish
Copy link
Member Author

This is no longer valid and was actually integrated in #5885 and then subsequently fixed in #7347 and #7353 so this no longer is necessary.

@mtreinish mtreinish closed this Jan 31, 2022
@mtreinish mtreinish deleted the update-options branch January 31, 2022 18:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: API Change Include in the "Changed" section of the changelog on hold Can not fix yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants