Skip to content
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

Support instead providing a list of values to write_memory() #1114

Merged
merged 10 commits into from
Jan 7, 2020
Merged

Support instead providing a list of values to write_memory() #1114

merged 10 commits into from
Jan 7, 2020

Conversation

tommoffat
Copy link
Contributor

@tommoffat tommoffat commented Nov 22, 2019

Description

Modified write_memory() for accepting lists in addition to values + offsets when writing to classical memory registers. Closes #658.

Checklist

  • The above description motivates these changes.
  • There is a unit test that covers these changes.
  • All new and existing tests pass locally and on Semaphore.
  • Parameters have type hints with PEP 484 syntax.
  • Functions and classes have useful sphinx-style docstrings.
  • (New Feature) The docs have been updated accordingly.
  • (Bugfix) The associated issue is referenced above using
    auto-close keywords.
  • The changelog is updated,
    including author and PR number (@username, gh-xxx).

Copy link
Contributor

@braised-babbage braised-babbage left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems reasonable, but I'd like a few sanity checks in the code for value type / length / etc.

pyquil/api/_qam.py Show resolved Hide resolved
pyquil/api/_qam.py Outdated Show resolved Hide resolved
Copy link
Contributor

@appleby appleby left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'll want to rebase onto master to resolve the make typecheck failure.

pyquil/api/_qam.py Outdated Show resolved Hide resolved
pyquil/api/_qam.py Outdated Show resolved Hide resolved
pyquil/api/_qam.py Show resolved Hide resolved
pyquil/api/_qam.py Outdated Show resolved Hide resolved
@notmgsk
Copy link
Contributor

notmgsk commented Dec 18, 2019

@Tommy-Moffat Does this need to be marked as a draft?

Copy link
Contributor

@appleby appleby left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

still needs rebase onto master to fix semaphore, but otherwise lgtm

@notmgsk notmgsk marked this pull request as ready for review December 18, 2019 18:10
@notmgsk notmgsk requested a review from a team as a code owner December 18, 2019 18:10
@rigetti rigetti deleted a comment from notmgsk Dec 18, 2019
@karalekas karalekas added the refactor 🔨 Rework existing functionality. label Dec 20, 2019
@karalekas karalekas added this to the v2.16 milestone Dec 20, 2019
Copy link
Contributor

@karalekas karalekas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple smol things and it's ready

pyquil/api/_qam.py Outdated Show resolved Hide resolved
pyquil/api/_qam.py Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
Tommy Moffat and others added 10 commits January 6, 2020 14:39
@karalekas karalekas merged commit f95b1f8 into rigetti:master Jan 7, 2020
@notmgsk
Copy link
Contributor

notmgsk commented Jan 7, 2020

🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor 🔨 Rework existing functionality.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

QAM.write_memory take a list rather than value + offset
6 participants