Opened 7 years ago

Closed 6 years ago

#19464 closed defect (fixed)

ExpressionTreeWalker fails on some functions

Reported by: Ralf Stephan Owned by:
Priority: major Milestone: sage-duplicate/invalid/wontfix
Component: symbolics Keywords:
Cc: Merged in:
Authors: Ralf Stephan Reviewers:
Report Upstream: N/A Work issues:
Branch: u/rws/expressiontreewalker_fails_on_some_functions (Commits, GitHub, GitLab) Commit: d2afc44b63a8247e70dba2a32d823cdb5c49c7bc
Dependencies: Stopgaps:

Status badges

Description (last modified by Ralf Stephan)

In some tickets (eg #15024, #16813) this doctest from symbolic/expression_conversions.py

            sage: foo = random_expr(20, nvars=2)
            sage: foo
            sage: s = ExpressionTreeWalker(foo)
            sage: bool(s() == foo)

fails because the set of functions returned by random_expr contains one of floor/ceil which currently don't accept the hold keyword:

sage: floor(x,hold=True)
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-8-fc5809e0a430> in <module>()
----> 1 floor(x,hold=True)

TypeError: __call__() got an unexpected keyword argument 'hold'

This would affect any use of the walker or its subclasses on floor expressions.

The reason is that both functions handle their calls themselves (instead of relying on superclass functionality) because at the time it was deemed necessary to provide a keyword named maximum_bits.

Change History (9)

comment:1 Changed 7 years ago by Ralf Stephan

Description: modified (diff)

comment:2 Changed 7 years ago by Ralf Stephan

Description: modified (diff)

comment:3 Changed 7 years ago by Ralf Stephan

Description: modified (diff)

comment:4 Changed 7 years ago by Ralf Stephan

Branch: u/rws/expressiontreewalker_fails_on_some_functions

comment:5 Changed 7 years ago by Ralf Stephan

Authors: Ralf Stephan
Commit: 4198c28765690dca3ee6ae80d3f9a7f021f3d919
Status: newneeds_review

The changes make floor/ceil accept the hold keyword and hand it over to the superclasses. It has no visible effect except fixing the bug, at the moment.


New commits:

4198c28fix 19464 by allowing a hold keyword on floor/ceil

comment:6 Changed 7 years ago by git

Commit: 4198c28765690dca3ee6ae80d3f9a7f021f3d919d2afc44b63a8247e70dba2a32d823cdb5c49c7bc

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

d2afc4419464: fix bug

comment:7 Changed 6 years ago by Vincent Delecroix

There is an incompatibility with #12121 which also fixes the issue. The ticket completely removes the handmade __call__ for both floor and ceil.

comment:8 Changed 6 years ago by Ralf Stephan

Milestone: sage-6.10sage-duplicate/invalid/wontfix
Status: needs_reviewpositive_review

OK, duplicate.

comment:9 Changed 6 years ago by Volker Braun

Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.