Revert checking of abstract parameters in post init #3197
Merged
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
jenshnielsen
force-pushed
the
revert_post_init
branch
from
8420c02 to
0df969b
Compare
astafan8
approved these changes
Jul 12, 2021
Codecov Report
@@ Coverage Diff @@
## master #3197 +/- ##
==========================================
- Coverage 65.93% 65.91% -0.03%
==========================================
Files 218 218
Lines 28907 28888 -19
==========================================
- Hits 19060 19041 -19
Misses 9847 9847 |
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.
It turns out that this approach has unintended side effects when multiple inheritance is used. This will effectively generate init methods for any class that does not have an init method but inherits from something that has an init method. In multiple inheritance scenarios this may change the order in which init methods are called. See https://gist.github.com/jenshnielsen/c3f6457f3510d638722c58e1e163ddee for an example of how this can happen.