feat: Allow mypy to check untyped defs #165
Conversation
| def _export_to_qctrl_expanded_format( | ||
| self, | ||
| filename=None, | ||
| filename, |
There was a problem hiding this comment.
Update the docstring now that this is no longer optional
| self.maximum_rabi_rate | ||
| ] * self.number_of_segments | ||
| field_names = sorted(control_info.keys()) | ||
| with open(filename, "w", newline="") as file: |
There was a problem hiding this comment.
The value for newline here could use an explanatory comment (maybe just a link to https://docs.python.org/3/library/csv.html#id3?)
| def export_to_file( | ||
| self, | ||
| filename=None, | ||
| filename, |
There was a problem hiding this comment.
Sorry, did you mean the doc for filename?
Seems it was not optional in the doc.
There was a problem hiding this comment.
D'oh, sorry. I just assumed it was.
| coordinates : str, optional | ||
| Indicates the co-ordinate system requested. Must be one of | ||
| 'cylindrical', 'cartesian' or 'polar'; defaults to 'cylindrical' | ||
| 'cylindrical', 'cartesian' or 'polar'. Defaults to 'cylindrical'. |
There was a problem hiding this comment.
BTW, I don't think we have the polar option, shall we remove it?
There was a problem hiding this comment.
Yup, I'd guess that's what we used to call cylindrical.
| def _export_to_qctrl_expanded_format( | ||
| self, | ||
| filename=None, | ||
| filename, |
| self.maximum_rabi_rate | ||
| ] * self.number_of_segments | ||
| field_names = sorted(control_info.keys()) | ||
| with open(filename, "w", newline="") as file: |
| def export_to_file( | ||
| self, | ||
| filename=None, | ||
| filename, |
There was a problem hiding this comment.
Sorry, did you mean the doc for filename?
Seems it was not optional in the doc.
charmasaur
left a comment
charmasaur
left a comment
There was a problem hiding this comment.
LGTM, although feel free to remove "polar" too.
| coordinates : str, optional | ||
| Indicates the co-ordinate system requested. Must be one of | ||
| 'cylindrical', 'cartesian' or 'polar'; defaults to 'cylindrical' | ||
| 'cylindrical', 'cartesian' or 'polar'. Defaults to 'cylindrical'. |
There was a problem hiding this comment.
Yup, I'd guess that's what we used to call cylindrical.
| def export_to_file( | ||
| self, | ||
| filename=None, | ||
| filename, |
There was a problem hiding this comment.
D'oh, sorry. I just assumed it was.
To set
check_untyped_defstoTrue, we need to make sure not to change the type of variable within the function body.This PR is to fix that issue.
The main change is in
_qctrl_expanded_export_contentwhere the type of return valuecontrol_infodepends on thecoordinates. It is aListfor CSV andDictfor JSON. However, we can always makecontrol_infoas aDictand change the format is the_export_to_qctrl_expanded_format(which calls_qctrl_expanded_export_contentto getcontrol_info) which only save the data to file and therefore does not have a return type but only side effect.some other minor tweaks:
filenamemandatoryexport_to_fileto fix the type issue and remove the error-rasing partnew_ramsey_sequenceto fix the type issue