-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
[QILI-46] Implement instrument classes / feat #9
Conversation
Add qibo and pyyaml requirements
Add description of attributes
SettingsLoader to SettingsManager
Great. Like it.
I would say that we figure this out, when we integrate with either flux qubit simulator or another device (whatever comes first)
Don't do this for now. Just we have to think that, whenever we need it, then we can refactor.
OK, now I get it what you meant about the settings thing to ask it in the StackOverflow. Let's discuss it today, but as I was saying this will be restructured after integrating with qiboconnection.
|
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.
Some comments, but mostly done.
However, now that we know how to create mocks, I would consider splitting the tests into unit
and integration
, leave the current tests as integration, and create unit tests with mocked.
…rresponding settings class
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.
Super great job and with tests!!!
Added
instruments/
folder, which contains:Added inside the
settings/
folder:I also removed some __init__ imports and I made sure to never use relative imports in src files.
Note: The instruments classes contain all the necessary methods to connect, setup, upload, run and acquire a sequence. In comparison with the classes of qibolab, these classes do not contain the methods used to:
Subnote: Since I created this branch from the
platform
branch, all the commits from the aforementioned branch are listed below... 😩Subsubnote: I had to delete
src
from theinterrogate
args inside the.pre-commit-config.yaml
file to avoid checking unstaged files.