Hack in SimpleNamespace-like functionality to Options#7347
Merged
mergify[bot] merged 3 commits intoDec 2, 2021
Conversation
This is an awful hack to support various SimpleNamespace-like operations that qiskit-dynamics are using. We need to transition downstream packages to the new Options API, but in the meantime, this makes the Options class behave as much like a SimpleNamespace as it can, while still keeping the new instance attributes and methods.
jakelishman
requested review from
a team,
chriseclectic and
jyu00
as code owners
mtreinish
added
Changelog: None
mtreinish
previously approved these changes
Dec 2, 2021
Member
mtreinish
left a comment
mtreinish
left a comment
There was a problem hiding this comment.
This LGTM, thanks for figuring all these hairy things out. Before we tag it automerge and put it in the merge queue though we should verify it works for qiskit-experiments. @eggerdj @nkanazawa1989 @chriseclectic if you could test this that would be great.
Pull Request Test Coverage Report for Build 1532598027
💛 - Coveralls |
chriseclectic
previously approved these changes
Dec 2, 2021
Member
chriseclectic
left a comment
chriseclectic
left a comment
There was a problem hiding this comment.
I ran qiskit-experiments tests locally with this branch and had no failures
I was surprised at how little pylint complained about the actual implementation, but it complains about the usage. This could conceivably be called a pylint bug; because we override `__setattr__`, pylint should perhaps recognise that general attribute setting might be allowed.
jakelishman
dismissed stale reviews from chriseclectic and mtreinish
via
d6ab60f
kdk
reviewed
Dec 2, 2021
Member
Author
|
It's possible that |
mtreinish
approved these changes
Dec 2, 2021
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This is an awful hack to support various SimpleNamespace-like operations
that qiskit-experiments are using. We need to transition downstream
packages to the new Options API, but in the meantime, this makes the
Options class behave as much like a SimpleNamespace as it can, while
still keeping the new instance attributes and methods.
Details and comments
This passes the
qiskit-experimentstests on my machine. The real problem is howqiskit-experimentsaccessesoptions.__dict__, so this involves some terrible behaviour to make that work as expected.It's an absolutely horrific hack, and I regret ever learning Python.
@eggerdj - this passes the tests on my machine, but could you check it does everything you need as well?