Conversation
…andling; 3. added more unit tests (with juliawgraham)
…in Match node; 3. added corresponding unit tests Co-authored-by: Lucybean-hi <Lucybean-hi@users.noreply.github.com> Co-authored-by: juliawgraham <juliawgraham@users.noreply.github.com> Co-authored-by: Erica Fu <erica-w-fu@users.noreply.github.com>
Co-authored-by: Yuqi <Yuqi07@users.noreply.github.com> Co-authored-by: Erica Fu <erica-w-fu@users.noreply.github.com> Co-authored-by: Lucybean-hi <Lucybean-hi@users.noreply.github.com>
MatchOr and MatchAnd
…rds and matchor) Co-authored-by: Yuqi <Yuqi07@users.noreply.github.com> Co-authored-by: Erica Fu <erica-w-fu@users.noreply.github.com> Co-authored-by: Lucybean-hi <Lucybean-hi@users.noreply.github.com>
|
Thanks, could you follow the recent changes in main? |
Could you clarify what those recent changes are? |
Your branch is behind the main branch. Can be resolved by merging main into this branch and resolving conflicts. |
Merging in new commits made one latexify main branch
Merging new main code into integration
|
We updated functions based on your suggestions, resolved all conflicts with the current main, and fixed all CI errors. Please take a check. |
| + r" \end{array} \right." | ||
| ) | ||
|
|
||
| latex_final = latex.replace("subject_name", subject_latex) |
There was a problem hiding this comment.
Weird hack, might cause issues if there's an actual subject_name in the
There was a problem hiding this comment.
Yes that is something we considered as well and would love to hear suggestions.
Since each of the visit_Match... functions cannot access the subject_latex variable(each of those functions can only have the ast node and pattern as the input), we had to find a workaround. We tried to use python format and % to insert the subject_latex but the use of % and {} in latex made this very difficult. Do you have any suggestions for how to better implement this (or possible a better name than "subject_name" to replace)
There was a problem hiding this comment.
Ah yeah I see the dilemma, bit perplexing lol. Not entirely sure how to solve but I'll give it some thought. Thanks for tackling match statements btw! Really cool feature :)
There was a problem hiding this comment.
Not sure what @odashi thinks of this solution, but you could create a stack of _match_subject_stack: list[str] = [], and at the very beginning of visit_Match do something like
subject_latex = self._expression_codegen.visit(node.subject)
self._match_subject_stack.append(subject_latex)and then right before you return do something like
latex = (
r"\left\{ \begin{array}{ll} "
+ r" \\ ".join(case_latexes)
+ r" \end{array} \right."
)
self._match_subject_stack.pop()
return latexand you would also have
def visit_MatchValue(self, node: ast.MatchValue) -> str:
"""Visit a MatchValue node."""
latex = self._expression_codegen.visit(node.value)
return self._match_subject_stack[-1] + " = " + latexA stack is only required if nested match statements is supported, which it seems like it is, although I have no idea if it's valid visit_MatchValue, the LHS takes the appropriate _subject_latex, i.e. the inner one should take the inner subject while the outer one takes the outer.
edit: _subject_latexes -> _match_subject_stack
There was a problem hiding this comment.
I think the @ZibingZhang 's solution is desirable at this point.
One minor point is that the private member _subject_latexes sounds weird for every other visitors, more specific name (like _match_subject_stack) would be better.
Co-authored-by: Zibing Zhang <44979059+ZibingZhang@users.noreply.github.com>
Co-authored-by: Zibing Zhang <44979059+ZibingZhang@users.noreply.github.com>
Co-authored-by: Zibing Zhang <44979059+ZibingZhang@users.noreply.github.com>
Co-authored-by: Zibing Zhang <44979059+ZibingZhang@users.noreply.github.com>
Co-authored-by: Zibing Zhang <44979059+ZibingZhang@users.noreply.github.com>
Co-authored-by: Zibing Zhang <44979059+ZibingZhang@users.noreply.github.com>
| + r" \end{array} \right." | ||
| ) | ||
|
|
||
| latex_final = latex.replace("subject_name", subject_latex) |
There was a problem hiding this comment.
I think the @ZibingZhang 's solution is desirable at this point.
One minor point is that the private member _subject_latexes sounds weird for every other visitors, more specific name (like _match_subject_stack) would be better.
odashi
left a comment
There was a problem hiding this comment.
Thanks for your contribution. Since the new change introduced other issues, I will take over this branch to merge it ASAP.
|
@odashi if you're busy would you mind if i looked into this for merging? I think a lot of the conflicts were due to my changes |
|
@ZibingZhang Yeah please! |
MatchOr and MatchAnd and implemented and tested.
Overview
All of these produce the correct latex code when a user enters a MatchOr case or guard statements in a python match statement.
Details
made edits to visit_Match
created a new function visit_MatchOr
We worked with four collaborators
@erica-w-fu
@Lucybean-hi
@juliawgraham
@Yuqi07
References
#93
Blocked by
#146
#147