Feature/Added explicit casting support#205
Conversation
Added new test cases for explicit casting
Added explicit casting logic in evaluate_expression function
Added explicit casting logic flow and cleanup the repeated code
Codecov ReportAttention: Patch coverage is
📢 Thoughts on this report? Let us know! |
src/pyqasm/visitor.py
Outdated
|
|
||
| return openqasm_bits | ||
|
|
||
| def _check_type_size( |
There was a problem hiding this comment.
This can be moved to the Qasm3Validator class as it has minimal dependency on the visitor and is only validating the size
There was a problem hiding this comment.
we can do that, but the problem is circular import.
because, _check_type_size is in expressions.py, which is trying to import Qasm3Validator from validator.py, But validator.py is also importing from expressions.py before it's fully loaded.
one simple way is, local import inside the function. can i do that..?
There was a problem hiding this comment.
Local imports for circumventing circular imports aren't really good practice. Usually it is possible to find a way to re-factor code to avoid circular imports.
There was a problem hiding this comment.
yeah, instead we can move them to expressions.py as a static methods. can i do that...?
src/pyqasm/visitor.py
Outdated
| return base_size | ||
|
|
||
| # pylint: disable-next=too-many-arguments | ||
| def _check_type_for_cast( |
There was a problem hiding this comment.
Similar comment as above, can be moved to the validator class
There was a problem hiding this comment.
same as above, because _check_type_for_cast is calling _check_type_size. we can place a local import inside _check_type_for_cast to avoid circular import problem.
There was a problem hiding this comment.
It'll be great if you can find a way to resolve this (and above) before Monday , June 16th (when the final submissions for unitary hack are due). But since it is a code re-factor issue, and not related to core functionality, we can tackle it later as well in a separate issue.
There was a problem hiding this comment.
Hi @vinayswamik , thank you for the great work! I have left some comments regarding organization of the code but overall it looks good.
Can you please also add a note in the CHANGELOG and an entry for the Cast in pyqasm supported instructions ?
|
Hi @vinayswamik , any reasons why you closed the PR? |
oops!, sorry that was by mistake, i deleted wrong branch from my side, so its auto closed. Now i reopened it. |
Added Cast to pyqasm supported operations
Performed minor cleanups to improve readability and maintainability.
Performed minor cleanups to improve readability and maintainability.
Added new and all possible test cases.
TheGupta2012
left a comment
TheGupta2012
left a comment
There was a problem hiding this comment.
Thank you @vinayswamik for the changes, LGTM!
3 participants
Summary of changes
This PR adds initial support for Explicit Casting in both
QasmVisitorandQasm3ExprEvaluator, in accordance with the OpenQASM 3 casting rules.With this change, we introduce logic to:
ValidationError.Closes #10