Skip to content

Fix: Correctly apply the phase in quilwaveforms#1408

Merged
notmgsk merged 4 commits intorigetti:rcfrom
bramathon:patch-2
Nov 19, 2021
Merged

Fix: Correctly apply the phase in quilwaveforms#1408
notmgsk merged 4 commits intorigetti:rcfrom
bramathon:patch-2

Conversation

@bramathon
Copy link
Collaborator

@bramathon bramathon commented Nov 17, 2021 •
edited by dbanty
Loading

See issue #1407: #1407

Description

Insert your PR description here. Thanks for contributing to pyQuil! 🙂

Checklist

  • The PR targets the rc branch (not master).
  • Commit messages are prefixed with one of the prefixes outlined in the commit syntax checker (see pattern field).
  • The above description motivates these changes.
  • There is a unit test that covers these changes.
  • All new and existing tests pass locally and on the PR's checks.
  • Parameters and return values have type hints with PEP 484 syntax.
  • Functions and classes have useful Sphinx-style docstrings.
  • All code follows Black style and obeys flake8 conventions.
  • (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, Test ignore #1234).

@bramathon bramathon requested a review from a team as a code owner November 17, 2021 23:02
@dbanty dbanty changed the title Update quilwaveforms to correctly apply the phase Fix: Correctly apply the phase in quilwaveforms Nov 18, 2021
@dbanty dbanty linked an issue Nov 18, 2021 that may be closed by this pull request
Phase is set to scale in update_envelope in several functions #1407
Closed
2 tasks
dbanty
dbanty approved these changes Nov 18, 2021
View reviewed changes
Copy link
Contributor

@dbanty dbanty left a comment

Choose a reason for hiding this comment

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

This looks good to me, thanks for the fix! Would love @notmgsk to take a quick glance before merge though.

@dbanty dbanty changed the base branch from master to rc November 18, 2021 16:58
genos
genos approved these changes Nov 18, 2021
View reviewed changes
Copy link
Contributor

@genos genos left a comment

Choose a reason for hiding this comment

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

Thanks for fixing these typos!

@dbanty
Copy link
Contributor

dbanty commented Nov 18, 2021

@bramathon could you just add a line to the changelog describing the fix? Thanks!

notmgsk
notmgsk approved these changes Nov 19, 2021
View reviewed changes
Copy link
Contributor

@notmgsk notmgsk left a comment

Choose a reason for hiding this comment

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

@dbanty failing on commit message due to having merged master -- why was that necessary?

@bramathon
Copy link
Collaborator Author

bramathon commented Nov 19, 2021

@dbanty failing on commit message due to having merged master -- why was that necessary?

Does this mean I need to change my commit messages to have these prefixes?

@dbanty
Copy link
Contributor

dbanty commented Nov 19, 2021

Don't worry about that check, we should take it off everywhere except master since we squash and merge PRs with the correct prefix (per contribution guide)

@notmgsk notmgsk merged commit 4305000 into rigetti:rc Nov 19, 2021
@notmgsk
Copy link
Contributor

notmgsk commented Nov 19, 2021

Thanks for the fix @bramathon!

@rigetti-githubbot
Copy link

rigetti-githubbot commented Nov 19, 2021

🎉 This PR is included in version 3.1.0-rc.4 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@genos genos mentioned this pull request Nov 24, 2021
Closed
2 tasks
dbanty added a commit that referenced this pull request Feb 14, 2022
@bramathon @dbanty
Fix: Correctly apply the phase in quilwaveforms (#1408)
22a2b37 
* Update quilwaveforms to correctly apply the phase

See issue #1407: #1407

* Update CHANGELOG.md

Co-authored-by: Dylan Anthony <43723790+dbanty@users.noreply.github.com>
@rigetti-githubbot
Copy link

rigetti-githubbot commented Feb 14, 2022

🎉 This PR is included in version 3.2.0-rc.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@rigetti-githubbot
Copy link

rigetti-githubbot commented Aug 3, 2022

🎉 This PR is included in version 3.2.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

3 more reviewers

@genos genos genos approved these changes

@notmgsk notmgsk notmgsk approved these changes

@dbanty dbanty dbanty approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

released on @rc released

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Phase is set to scale in update_envelope in several functions

5 participants

@bramathon @dbanty @notmgsk @rigetti-githubbot @genos