-
Notifications
You must be signed in to change notification settings - Fork 322
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
Raise on get/set of Abstract parameters, rework example notebook #3217
Merged
jenshnielsen
merged 8 commits into
microsoft:master
from
jenshnielsen:abstract_notebook_move
Jul 19, 2021
Merged
Raise on get/set of Abstract parameters, rework example notebook #3217
jenshnielsen
merged 8 commits into
microsoft:master
from
jenshnielsen:abstract_notebook_move
Jul 19, 2021
Conversation
This file contains 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
Collaborator
jenshnielsen
commented
Jul 19, 2021
•
edited
Loading
edited
- Move notebook to a subdir where it belongs
- Raise on get/set of abstract parameters (until we can re enable the error on init) add a test too
- Remove sections about error on init from notebook
- Fix a typo in the title of the notebook
- Enable execution of notebook so this can be avoided in the future
b6c3ae4
to
a7bab03
Compare
a7bab03
to
4dc791d
Compare
astafan8
approved these changes
Jul 19, 2021
Codecov Report
@@ Coverage Diff @@
## master #3217 +/- ##
=======================================
Coverage 65.92% 65.92%
=======================================
Files 218 218
Lines 28939 28943 +4
=======================================
+ Hits 19078 19082 +4
Misses 9861 9861 |
This was referenced Dec 21, 2021
bors bot
added a commit
that referenced
this pull request
Jan 8, 2022
3718: Reimplement abstract instrument r=jenshnielsen a=jenshnielsen This is a start of re implementing the abstract parameter interface reverted in #3197 and #3217 based on top of #3696 Compared to the solution based on init subclass this works a bit differently. The metaclass in use is only implemented for the Instrument class but not for the InstrumentChannel. Changing this would require metaclass inheritance which is a bad idea. This meas that it is not an error to create an InstrumentChannel with abstract parameters but it is an error to attach such a thing to an instrument at instrument creation time. I don't think this is a big issue since that is how most instruments are created. Still need to update docs again and verify if there are other reverted changes to bring over Co-authored-by: Jens H. Nielsen <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.