-
Notifications
You must be signed in to change notification settings - Fork 30
Linspace followup waveforms #920
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
base: master
Are you sure you want to change the base?
Conversation
Test Results 6 files 6 suites 7m 19s ⏱️ Results for commit 60363e7. ♻️ This comment has been updated with latest results. |
|
||
|
||
@property | ||
def _pow_2_divisor(self) -> int: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CAn we move this to metadata?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
makes sense; forgot that this is already implemented.
like this or even without accessor?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we do not need to include it in the type system for now. Just add it as a dynamic property.
Question is, how to handle it for the waveform. whould we add a metadata field there as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
possibly, but the waveforms are not really used in the public interface anyway, so yagni?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We will need it to propagate properties like these.
qupulse/program/waveforms.py
Outdated
@@ -1280,3 +1281,71 @@ def reversed(self) -> 'Waveform': | |||
|
|||
def __repr__(self): | |||
return f"ReversedWaveform(inner={self._inner!r})" | |||
|
|||
|
|||
class WaveformCollection(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need WaveformCollection in the main repo?
I thnk it needs a different name and it belongs into a different file. I understand what the class is used for but I do not have a concept of what it actually represents (in terms of nesting). I think it is useful for general command table like devices, but needs more refinement before putting it into the main repo.
with the infamous power 2 hack, but with a test which works