Opened 5 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: |
Description (last modified by )
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 5 years ago by
- Description modified (diff)
comment:2 Changed 5 years ago by
- Description modified (diff)
comment:3 Changed 5 years ago by
- Description modified (diff)
comment:4 Changed 5 years ago by
- Branch set to u/rws/expressiontreewalker_fails_on_some_functions
comment:5 Changed 5 years ago by
- Commit set to 4198c28765690dca3ee6ae80d3f9a7f021f3d919
- Status changed from new to needs_review
comment:6 Changed 5 years ago by
- Commit changed from 4198c28765690dca3ee6ae80d3f9a7f021f3d919 to d2afc44b63a8247e70dba2a32d823cdb5c49c7bc
Branch pushed to git repo; I updated commit sha1. New commits:
d2afc44 | 19464: fix bug
|
comment:7 Changed 5 years ago by
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
- 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
- Resolution set to fixed
- Status changed from positive_review to closed
The changes make
floor
/ceil
accept thehold
keyword and hand it over to the superclasses. It has no visible effect except fixing the bug, at the moment.New commits:
fix 19464 by allowing a hold keyword on floor/ceil