Check for multiples of pi at operator construction time

[notmgsk]

Rather than trying to figure it out at the time of printing.

Closes #943.

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

[ecpeterson]

One concrete comment. Also, one nebulous concern: is there any danger in adding str to the possible types of the operands for Mul and Div and then using the resulting trees (say, in pyQVM) to evaluate parameters? Do all interested parties know how to convert "pi" back out to pi?

[ecpeterson]

update the docstring to reflect the return type

[notmgsk]

One concrete comment. Also, one nebulous concern: is there any danger in adding str to the possible types of the operands for Mul and Div and then using the resulting trees (say, in pyQVM) to evaluate parameters? Do all interested parties know how to convert "pi" back out to pi?

Does pyQVM support parameteric programs? Even so, I should better understand the parameter model and make sure I'm not polluting it.

[karalekas]

@notmgsk you're going to want to pull this branch when you start working again. also, can you come up with a PR title that is a bit shorter (~50 chars)? thanks!

[notmgsk]

One concrete comment. Also, one nebulous concern: is there any danger in adding str to the possible types of the operands for Mul and Div and then using the resulting trees (say, in pyQVM) to evaluate parameters? Do all interested parties know how to convert "pi" back out to pi?

In retrospect, I don't think this is an issue. Expressions already consumed strings, and it hasn't been an issue thus far. I don't think this is the most pleasant solution, but I also don't want to this to snowball into a much larger rewrite. Closing #995 will probably ease some concern.

[appleby]

One concrete comment. Also, one nebulous concern: is there any danger in adding str to the possible types of the operands for Mul and Div and then using the resulting trees (say, in pyQVM) to evaluate parameters? Do all interested parties know how to convert "pi" back out to pi?

In retrospect, I don't think this is an issue. Expressions already consumed strings, and it hasn't been an issue thus far. ...

Over in my type hints PR, I assumed they only consumed Union[Expression, int, float, complex]. I certainly might have got the type wrong. Do you have an example in mind of when they take a string? I assumed that all parameters or memory references would already be boxed into the corresponding Parameter and MemoryReference classes by the time an Expression sees them.

[notmgsk]

One concrete comment. Also, one nebulous concern: is there any danger in adding str to the possible types of the operands for Mul and Div and then using the resulting trees (say, in pyQVM) to evaluate parameters? Do all interested parties know how to convert "pi" back out to pi?

In retrospect, I don't think this is an issue. Expressions already consumed strings, and it hasn't been an issue thus far. ...

Over in my type hints PR, I assumed they only consumed Union[Expression, int, float, complex]. I certainly might have got the type wrong. Do you have an example in mind of when they take a string? I assumed that all parameters or memory references would already be boxed into the corresponding Parameter and MemoryReference classes by the time an Expression sees them.

Sure: Div("pi")

[appleby]

One concrete comment. Also, one nebulous concern: is there any danger in adding str to the possible types of the operands for Mul and Div and then using the resulting trees (say, in pyQVM) to evaluate parameters? Do all interested parties know how to convert "pi" back out to pi?

In retrospect, I don't think this is an issue. Expressions already consumed strings, and it hasn't been an issue thus far. ...

Over in my type hints PR, I assumed they only consumed Union[Expression, int, float, complex]. I certainly might have got the type wrong. Do you have an example in mind of when they take a string? I assumed that all parameters or memory references would already be boxed into the corresponding Parameter and MemoryReference classes by the time an Expression sees them.

Sure: Div("pi")

You can pass the Div constructor anything because it doesn't check the types.

In [139]: pq.quilatom.Div(range(3), range(3))
Out[148]: Div(range(0, 3),range(0, 3))

What I meant was are there any examples where a BinaryExpr actually winds up getting an unboxed string in current pyquil usage. I will do another grep search. Previously I was trying to be clever and grep for occurrence with string literals, but I should probably just do an exhaustive search. Hopefully there aren't too many places where these get instantiated...

[notmgsk]

Closing out of frustration. The solution is ugly and I can't decide whether it will break other stuff. Will return another day.