Closed PiergiorgioButtarini closed 7 months ago
Attention: 2 lines
in your changes are missing coverage. Please review.
Comparison is base (
92f35df
) 62.45% compared to head (dfef079
) 62.45%. Report is 47 commits behind head on main.
Files | Patch % | Lines |
---|---|---|
src/qibolab/instruments/qblox/controller.py | 66.66% | 2 Missing :warning: |
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
does not strongly violate the naming convention (for that I am not sure though)
Indeed, as I said above, acronyms involve a certain degree of arbitrariness
But generally, excluding extremely bad choices (eg.
class cluster_qcm_bb
), I don't have strong opinion because no matter how intuitive we try to make names, most users will still need to look at the docs/code to know what to import.
For sure.
I would avoid even the current one, because still wrong (because of the _
) even if mildly wrong. For the rest is a matter of taste, I will leave the choice to @PiergiorgioButtarini :)
Instruments are now working and QPU tests are passing, so we can probably merge this if the names are final.
I agree with @stavros11, this could be merged. It's a relatively small (and primarily internal) change, that has been already propagated consistently.
Qblox modules now have conventional names. Also I removed a deprecated test file.
Closes #742 Checklist: