Opened 6 years ago

Closed 5 years ago

#19464 closed defect (fixed)

ExpressionTreeWalker fails on some functions

Reported by: rws 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 rws)

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

            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 5 years ago by rws

  • Description modified (diff)

comment:2 Changed 5 years ago by rws

  • Description modified (diff)

comment:3 Changed 5 years ago by rws

  • Description modified (diff)

comment:4 Changed 5 years ago by rws

  • Branch set to u/rws/expressiontreewalker_fails_on_some_functions

comment:5 Changed 5 years ago by rws

  • Authors set to Ralf Stephan
  • Commit set to 4198c28765690dca3ee6ae80d3f9a7f021f3d919
  • Status changed from new to needs_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 5 years ago by git

  • Commit changed from 4198c28765690dca3ee6ae80d3f9a7f021f3d919 to d2afc44b63a8247e70dba2a32d823cdb5c49c7bc

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

d2afc4419464: fix bug

comment:7 Changed 5 years ago by vdelecroix

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 5 years ago by rws

  • Milestone changed from sage-6.10 to sage-duplicate/invalid/wontfix
  • Status changed from needs_review to positive_review

OK, duplicate.

comment:9 Changed 5 years ago by vbraun

  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.