-
Notifications
You must be signed in to change notification settings - Fork 25
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
Access p-type variables #47
Conversation
Codecov Report
@@ Coverage Diff @@
## master #47 +/- ##
==========================================
+ Coverage 97.18% 97.30% +0.12%
==========================================
Files 12 12
Lines 1915 1929 +14
==========================================
+ Hits 1861 1877 +16
+ Misses 54 52 -2
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice update, looks good from my end @thisac!
are otherwise stored in `prog.variables`. This also changes TDM programs to not be templates | ||
(`prog.is_template` now returns `False`). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just out of curiosity - is there a consequence of TDM programs not being templates any more?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think so, no. As far as I know this is just a consequence of how we decided to implement these p-type parameters internally. It only blocks the use of the match_template
function for all TDM programs (this fixes that).
@@ -561,7 +561,8 @@ def exitProgram(self, ctx: blackbirdParser.ProgramContext): | |||
self._program._var.update(_VAR) | |||
_VAR.clear() | |||
|
|||
self._program._parameters.extend(_PARAMS) | |||
is_ptype = lambda p: str(p)[0] == "p" and str(p)[1:].isdigit() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very minor suggestion, but you could define this function at the module level, since it doesn't seem to depend on closure :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good from my end @thisac!
def is_ptype(p): | ||
"""Checks whether a parameter is in the form `p0`, `p1`, etc.""" | ||
return str(p)[0] == "p" and str(p)[1:].isdigit() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feels much cleaner this way :)
Co-authored-by: Josh Izaac <josh146@gmail.com>
prog.variables
match_template
to automatically return matched parameters with their array values, rather than their array names (p0
,p1
, etc.)prog.parameters
since they're only needed there internally and are otherwise stored inprog.variables
. This also changes TDM programs to not be templates (prog.is_template
now returnsFalse
).