/get targets refactor
#788
Conversation
| """A class to store data returned from a /get_targets request.""" | ||
|
|
||
| target: str | ||
| properties: Dict[str, bool] |
There was a problem hiding this comment.
can we flatten this so instead of properties we have e.g. TargetInfo.available?
this also might be a really place to start using pydantic - instead of a dataclass, we could use use
class TargetInfo(pydantic.BaseModel):
"""A class to store data returned from a /get_targets request."""
target: str
available: bool = False
...so that we could instantiate it with e.g.
TargetInfo(target=target, **properties)(and it will automatically handle defaults/validation/etc for us)
There was a problem hiding this comment.
Like the idea of using pydantic, will update. As for the decision to use properties, it was because I thought that if more fields were added, we wouldn't need to create a new variable for each one in the original dataclass. I was thinking that the user wouldn't use the TargetInfo dataclass directly, but instead would be able to do get_targets(available=True) or any keyword supported in properties
store TargetInfo list
bharat-thotakura
force-pushed
the
get_targets_refactor
branch
from
6601472
to
89cb821
Compare
bharat-thotakura
requested review from
pgoiporia,
paaige,
vtomole and
epelaaez
as code owners
| available: bool = False | ||
| retired: bool = False | ||
|
|
||
| def __repr__(self) -> str: |
There was a problem hiding this comment.
worth confirming, but i think pydantic.BaseModel automatically defines this and __eq__ so we don't need to
There was a problem hiding this comment.
Yeah it does define __repr__ but its formatting looks like this (i.e., without the newlines from the dedicated one which I guess takes up more space):
[TargetInfo(target='aws_dm1_simulator', supports_submit=True, supports_submit_qubo=False, supports_compile=True, available=True, retired=False),
TargetInfo(target='aws_sv1_simulator', supports_submit=True, supports_submit_qubo=False, supports_compile=True, available=True, retired=False),
TargetInfo(target='aws_tn1_simulator', supports_submit=True, supports_submit_qubo=False, supports_compile=True, available=True, retired=False),
...
It does also handle the __eq__, I just had an assertion error in one of my earlier tests that was fixed by adding an __eq__ but that's not an issue anymore, so it's removed now.
use gss.TargetInfo import fix updated gss.testing import
Co-authored-by: richrines1 <85512171+richrines1@users.noreply.github.com>
bharat-thotakura
force-pushed
the
get_targets_refactor
branch
from
bb48cd5
to
6015453
Compare
There was a problem hiding this comment.
lgtm % deployment of server-side changes (and corresponding reversions here)!
Fixes Infleqtion/server-superstaq#2683. (`gss.testing.py` was added because it will be further used by #788)
Requires https://github.com/Infleqtion/server-superstaq/pull/2822 to test methods locally
2nd part to close out https://github.com/Infleqtion/server-superstaq/issues/2813.