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: sage-6.8
Component: symbolics Keywords:
Cc: Karl-Dieter 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:

Status badges

Description (last modified by Ralf Stephan)

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

Used in #9424, #14801, #17849

Change History (20)

comment:1 Changed 8 years ago by Ralf Stephan

Branch: public/17759

comment:2 Changed 8 years ago by Ralf Stephan

Commit: 3781eec1432c7efe6531be4338fef51cc04eda6d
Status: newneeds_review

New commits:

3781eec17759: convenience class symbolic ExpressionTreeWalker(Converter)

comment:3 Changed 8 years ago by Karl-Dieter Crisman

Cc: Karl-Dieter Crisman added

comment:4 Changed 8 years ago by Karl-Dieter Crisman

Cc: Niles Johnson added

comment:5 Changed 8 years ago by Ralf Stephan

Description: modified (diff)

comment:6 Changed 8 years ago by Ralf Stephan

Description: modified (diff)

comment:7 Changed 8 years ago by Ralf Stephan

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:

Last edited 8 years ago by Ralf Stephan (previous) (diff)

comment:8 Changed 8 years ago by Ralf Stephan

Status: needs_reviewneeds_work
Work issues: handle hold=True,hypergeometric

comment:9 Changed 8 years ago by git

Commit: 3781eec1432c7efe6531be4338fef51cc04eda6d16aa81d5f5fe7b38db0ce7372e198b6f036ee724

Branch pushed to git repo; I updated commit sha1. New commits:

9375510Merge branch 'develop' into t/17759/public/17759
16aa81d17759: handle hold=True and hypergeometric

comment:10 Changed 8 years ago by Ralf Stephan

Milestone: sage-6.5sage-6.6
Status: needs_workneeds_review
Work issues: handle hold=True,hypergeometric

comment:11 Changed 8 years ago by Ralf Stephan

Passes all tests in a 6.6rc1 patchbot run (ignore long startup, docbuild plugins).

comment:12 Changed 7 years ago by Ralf Stephan

Dependencies: 18255

Now some patchbot fails because of #18255.

comment:13 Changed 7 years ago by Ralf Stephan

Dependencies: 18255#18255
Milestone: sage-6.6sage-6.7

comment:14 Changed 7 years ago by Ralf Stephan

Branch: public/17759u/rws/17759

comment:15 Changed 7 years ago by Ralf Stephan

Commit: 16aa81d5f5fe7b38db0ce7372e198b6f036ee724fa73285534e56d5ccd19ffc943237757ea06bcf1
Milestone: sage-6.7sage-6.8

New commits:

fa7328517759: convenience class symbolic ExpressionTreeWalker(Converter)

comment:16 Changed 7 years ago by Ralf Stephan

Description: modified (diff)

comment:17 Changed 7 years ago by Marc Mezzarobba

Reviewers: Marc Mezzarobba
Status: needs_reviewpositive_review

Lgtm, though I don't know that code too well... (and the commit history is a little bit confusing :-))

comment:18 in reply to:  17 Changed 7 years ago by Ralf Stephan

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:19 Changed 7 years ago by Ralf Stephan

Ah no, my mistake, I didn't squash it all, sorry.

comment:20 Changed 7 years ago by Volker Braun

Branch: u/rws/17759fa73285534e56d5ccd19ffc943237757ea06bcf1
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.