add snapshot_id to get_sampler interface on engine object #7005
Conversation
senecameeks
requested review from
wcourtney,
vtomole,
verult and
a team
as code owners
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7005 +/- ##
==========================================
- Coverage 97.89% 97.89% -0.01%
==========================================
Files 1085 1085
Lines 95098 95108 +10
==========================================
+ Hits 93096 93104 +8
- Misses 2002 2004 +2 ☔ View full report in Codecov by Sentry. |
| self, | ||
| processor_id: Union[str, List[str]], | ||
| run_name: str = "", | ||
| snapshot_id: str = "", |
There was a problem hiding this comment.
for backwards compatiblity, shouldn't snapshot_id be last since it's a new arg?
There was a problem hiding this comment.
In #6739 , the reviewer suggestion was to put snapshot_id next to run_name since they are related. I'd like to keep the APIs consistent. I don't think our guarantees of backwards compatibility is stringent in cirq-google
There was a problem hiding this comment.
On second thought moving snapshot_id to the last arg is consistent with the get_sampler API on the EngineProcessor. Moving it
…Cirq into u/smeeks/engine_get_sampler
Fixes #7001