Opened 8 years ago
Closed 7 years ago
#17759 closed enhancement (fixed)
convenience class symbolic ExpressionTreeWalker(Converter)
Reported by:  Ralf Stephan  Owned by:  

Priority:  major  Milestone:  sage6.8 
Component:  symbolics  Keywords:  
Cc:  KarlDieter Crisman, Niles Johnson  Merged in:  
Authors:  Ralf Stephan  Reviewers:  Marc Mezzarobba 
Report Upstream:  N/A  Work issues:  
Branch:  fa73285 (Commits, GitHub, GitLab)  Commit:  fa73285534e56d5ccd19ffc943237757ea06bcf1 
Dependencies:  #18255  Stopgaps: 
Description (last modified by )
This class can already be used as base class for SubstituteFunction
but is intended also for other uses.
sage: from sage.symbolic.expression_conversions import ExpressionTreeWalker sage: from sage.symbolic.random_tests import random_expr sage: foo = random_expr(20, nvars=2) sage: s = ExpressionTreeWalker(foo) sage: bool(s() == foo) True
Change History (20)
comment:1 Changed 8 years ago by
Branch:  → public/17759 

comment:2 Changed 8 years ago by
Commit:  → 3781eec1432c7efe6531be4338fef51cc04eda6d 

Status:  new → needs_review 
comment:3 Changed 8 years ago by
Cc:  KarlDieter Crisman added 

comment:4 Changed 8 years ago by
Cc:  Niles Johnson added 

comment:5 Changed 8 years ago by
Description:  modified (diff) 

comment:6 Changed 8 years ago by
Description:  modified (diff) 

comment:7 Changed 8 years ago by
OK, this is not really a tree walker but a deep copier, i.e., function objects are called again. This misses possible hold arguments that were given but are lost, see #17849. So let's implement a real tree walker in this ticket that only replaces specific operators but leaves the rest of the tree as is.
EDIT:
comment:8 Changed 8 years ago by
Status:  needs_review → needs_work 

Work issues:  → handle hold=True,hypergeometric 
comment:9 Changed 8 years ago by
Commit:  3781eec1432c7efe6531be4338fef51cc04eda6d → 16aa81d5f5fe7b38db0ce7372e198b6f036ee724 

comment:10 Changed 8 years ago by
Milestone:  sage6.5 → sage6.6 

Status:  needs_work → needs_review 
Work issues:  handle hold=True,hypergeometric 
comment:11 Changed 8 years ago by
Passes all tests in a 6.6rc1 patchbot run (ignore long startup, docbuild plugins).
comment:13 Changed 7 years ago by
Dependencies:  18255 → #18255 

Milestone:  sage6.6 → sage6.7 
comment:14 Changed 7 years ago by
Branch:  public/17759 → u/rws/17759 

comment:15 Changed 7 years ago by
Commit:  16aa81d5f5fe7b38db0ce7372e198b6f036ee724 → fa73285534e56d5ccd19ffc943237757ea06bcf1 

Milestone:  sage6.7 → sage6.8 
New commits:
fa73285  17759: convenience class symbolic ExpressionTreeWalker(Converter)

comment:16 Changed 7 years ago by
Description:  modified (diff) 

comment:17 followup: 18 Changed 7 years ago by
Reviewers:  → Marc Mezzarobba 

Status:  needs_review → positive_review 
Lgtm, though I don't know that code too well... (and the commit history is a little bit confusing :)
)
comment:18 Changed 7 years ago by
Replying to mmezzarobba:
Lgtm, though I don't know that code too well... (and the commit history is a little bit confusing
:)
)
Changing the branch should make earlier commits disappear, at least in the commit log. Do we have a trac bug ticket for this?
But thanks for the review!
comment:20 Changed 7 years ago by
Branch:  u/rws/17759 → fa73285534e56d5ccd19ffc943237757ea06bcf1 

Resolution:  → fixed 
Status:  positive_review → closed 
New commits:
17759: convenience class symbolic ExpressionTreeWalker(Converter)