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

Jump to bottom

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

Merged
merged 3 commits into from Feb 9, 2022
Merged

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

merged 3 commits into from Feb 9, 2022
+33 −8

Conversation

caldwellshane
Copy link
Member

@caldwellshane caldwellshane commented Feb 9, 2022 •
edited by notmgsk

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, #1234).

@caldwellshane caldwellshane requested a review from as a code owner Feb 9, 2022
@caldwellshane
Copy link
Member Author

@caldwellshane caldwellshane commented Feb 9, 2022 •
edited

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

dbanty
dbanty reviewed Feb 9, 2022
View changes
pyquil/_parser/parser.py Show resolved Hide resolved
dbanty
dbanty approved these changes Feb 9, 2022
View changes
Copy link
Contributor

@dbanty dbanty left a comment

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

notmgsk
notmgsk reviewed Feb 9, 2022
View changes
test/unit/test_parser.py
assert (
fdef.out()
== r"""DEFFRAME My-Cool-Qubit "bananas":
DIRECTION: "go west"
Copy link
Contributor

@notmgsk notmgsk Feb 9, 2022

Choose a reason for hiding this comment

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

nice

notmgsk
notmgsk reviewed Feb 9, 2022
View changes
pyquil/_parser/grammar.lark
@@ -60,7 +60,7 @@ def_circuit : "DEFCIRCUIT" name [ variables ] [ qubit_designators ] ":" indented
// | "DEFCIRCUIT" name [ variables ] qubit_designators ":" indented_instrs -> def_circuit_qubits

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

@notmgsk notmgsk Feb 9, 2022

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
Member Author

@caldwellshane caldwellshane Feb 9, 2022

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
notmgsk approved these changes Feb 9, 2022
View changes
@notmgsk
Copy link
Contributor

@notmgsk notmgsk commented Feb 9, 2022

Hella tight.

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

@rigetti-githubbot rigetti-githubbot commented Feb 11, 2022

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

The release is available on GitHub release

Your semantic-release bot 📦🚀

@caldwellshane caldwellshane mentioned this pull request Feb 11, 2022
Closed
dbanty added a commit that referenced this issue Feb 14, 2022
@caldwellshane @dbanty
Fix: DefFrame to/from Quil with JSON values (#1419)
1727895 
* Fix: #1417 DefFrame to/from Quil with JSON values

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

@rigetti-githubbot 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 📦🚀

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

@notmgsk notmgsk

@dbanty dbanty

Assignees
No one assigned
Labels
released on @rc
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet
4 participants
@caldwellshane @notmgsk @rigetti-githubbot @dbanty