Add type hint for predefined driven control #102
Conversation
| "rabi_rotation angle must be either pi, pi/2 or pi/4", | ||
| {"rabi_rotation": rabi_rotation}, | ||
| ) | ||
| if rabi_rotation is not None: |
There was a problem hiding this comment.
Note that this only fixes the type checking, but still has a bug when rabi_rotation is None. That is, theta_3 and phi_3 will not be defined if rabi_rotation is None (which can be fixed by moving them to the if branch, but the there is still an issue with duration). Same bugs below.
In the tests, rabi_rotation was always provided and therefore missed this corner case.
Seems there are two options:
- handle the
Nonecase. It seems whenrabi_rotationisNone,durationwill also beNone. However, I can't see the point of having such driven controls. - does not allow 'rabi_rotation` to be optional (preferred)
There was a problem hiding this comment.
Sorry, I missed this comment
There was a problem hiding this comment.
NP, this is the same issue as you commented above. Now should be fixed with making rabi_rotation required.
| phi_p = _get_transformed_rabi_rotation_wimperis(rabi_rotation) | ||
|
|
||
| rabi_rotations = [rabi_rotation, np.pi, 2 * np.pi, np.pi] | ||
| if rabi_rotation is not None: |
There was a problem hiding this comment.
Any idea why the type checker isn't picking up that line 302 could fail if rabi_rotation is None, since phi_p won't get defined?
There was a problem hiding this comment.
Not sure. I was quite surprised to see such an obvious error just passes mypy checking.
There was a problem hiding this comment.
Crazy stuff.
Seems we have an agreement here, I'll just make it required. Making |
|
Make |
| rabi_rotation : float | ||
| The total polar angle to be performed by the pulse. | ||
| Defined in polar coordinates. | ||
| maximum_rabi_rate : float | ||
| Defined in polar coordinates. Defaults to None. |
There was a problem hiding this comment.
Remove the comment about defaulting to None
Private methods like
_new_***allowrabi_rotationto be optional but they all call_predefined_common_attributeswhich does not allowrabi_rotationto beNone.Some refactoring is done to make sure the code pass type checking.
Edit: note that this is only to fix the type issue, but does not fix the bug when user doesn't explicitly set
rabi_rotation.