non-recursive SameQ[] #1077
Merged
non-recursive SameQ[] #1077
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@mmatera The idea is interesting and needed. I'd like look over this in more detail over the weekend and think about it. Thanks! |
rocky
reviewed
Aug 31, 2024
mathics/doc/latex/mathics-title.pdf
Outdated
There was a problem hiding this comment.
@mmatera Something in "make clean" is removing this. I haven't found where or fixed this.
Also it would be nice if we had a way to automatically generate ths.
mmatera
force-pushed
the
sameQ_nonrecursive
branch
from
c9a8f62 to
16f2eda
Compare
@mmatera looked the code over and do not find anything wrong with it. I made some small changes to the docstring comment and addressed some of the lint errors my editor is telling me about. We can't use generators here since there we access and then "rewind" sometimes. And this _is_ essentially a version without the tail recursions. So I've removed the comments concerning doubt. If you have the example that caused the failure on 3.12 without this change, it would be interesting to benchmark just this test inside a Mathics3 session to see if there's any difference in execution time. Other comments I'll put in the specific code changes in this PR. --------- Co-authored-by: Juan Mauricio Matera <[email protected]>
|
LGTM - merge when you are ready. |
|
This is what I get when I run
For some reason, it does not happen now in Github Actions |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is a proposal for a reimplementation of
Expression.sameQwhich uses a tree transversal iterative algorithm instead of recursion. Using this implementation, we avoid the recursion issue in the Pytests for Python 3.12.I leave this as a draft for a while, in case someone else wants to propose a better solution.