Skip to content

Fix: #1417 DefFrame to/from Quil with JSON values#1419

Merged
notmgsk merged 3 commits intorigetti:rcfrom
caldwellshane:1417-fix-defframe-tofrom-quil-with-json
Feb 9, 2022
Merged

Fix: #1417 DefFrame to/from Quil with JSON values#1419
notmgsk merged 3 commits intorigetti:rcfrom
caldwellshane:1417-fix-defframe-tofrom-quil-with-json

Conversation

@caldwellshane
Copy link
Contributor

@caldwellshane caldwellshane commented Feb 9, 2022

Description

Closes #1417, #1418

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).

@caldwellshane caldwellshane requested a review from a team as a code owner February 9, 2022 06:20
@caldwellshane
Copy link
Contributor Author

caldwellshane commented Feb 9, 2022

@notmgsk This is ready for a look when you're up.

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.

LGTM, though you might still want @notmgsk 's approval since he knows Quil much better than me.

assert (
fdef.out()
== r"""DEFFRAME My-Cool-Qubit "bananas":
DIRECTION: "go west"
Copy link
Contributor

Choose a reason for hiding this comment

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

nice


def_frame : "DEFFRAME" frame ( ":" frame_spec+ )?
frame_spec : _NEWLINE_TAB frame_attr ":" (expression | string )
frame_spec : _NEWLINE_TAB frame_attr ":" ( expression | string )
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, so it already was string. I must've been looking at the wrong/old branch.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, we were looking at master, but this was already done on rc. When I got here and saw it I assumed you'd done it. :)

@notmgsk
Copy link
Contributor

notmgsk commented Feb 9, 2022

Hella tight.

@notmgsk notmgsk merged commit f9608d8 into rigetti:rc Feb 9, 2022
@caldwellshane caldwellshane deleted the 1417-fix-defframe-tofrom-quil-with-json branch February 9, 2022 22:20
@rigetti-githubbot
Copy link

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

The release is available on GitHub release

Your semantic-release bot 📦🚀

dbanty pushed a commit that referenced this pull request Feb 14, 2022
* Fix: #1417 DefFrame to/from Quil with JSON values

* No code: Update changelog
@rigetti-githubbot
Copy link

🎉 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

🎉 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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants