-
Notifications
You must be signed in to change notification settings - Fork 33
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Client + docs: rename Program
#1057
Conversation
f545d87
to
fc28254
Compare
fc28254
to
4011d6a
Compare
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.
Should this also get tagged as a feature branch? there's a new class being added / an old one being deprecated, so technically ... (?)
"job" | ||
] | ||
}, | ||
{ | ||
"cell_type": "code", | ||
"execution_count": 7, | ||
"execution_count": null, |
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.
do you want to execute this cell? (think there might also one other in this notebook that wasn't executed...)
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.
I left a couple of comments and I found a couple of references in the documentation that maybe we can review but I would merge it and do next week another pass, what do you think, @IceKhan13 ?
|
||
# create program | ||
program = Program( | ||
program = QiskitPattern( |
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.
Should we rename variable program
to pattern
too?
pattern = QiskitPattern(
@@ -166,11 +168,13 @@ def run(self, program: Program, arguments: Optional[Dict[str, Any]] = None): | |||
) | |||
return Job(job_id=job_id, job_client=self) | |||
|
|||
def upload(self, program: Program): | |||
def upload(self, program: QiskitPattern): |
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.
This can a breaking change so it's a bit tricky but should we call it pattern
?
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.
can you overload functions in python? (i.e. have one that takes a program and mark it deprecated and another that takes a pattern)
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.
good question!
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.
seems like answer is no (though there are a few workarounds if you search around on the topic...)
This is more of a docs change + name alias. It does not change any functionality |
We will follow with updates in next PR :) |
Summary
Client + docs: rename
Program