Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ExpressionTreeWalker fails on some functions #19464

Closed
rwst opened this issue Oct 24, 2015 · 11 comments
Closed

ExpressionTreeWalker fails on some functions #19464

rwst opened this issue Oct 24, 2015 · 11 comments

Comments

@rwst
Copy link
Contributor

rwst commented Oct 24, 2015

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.

Component: symbolics

Author: Ralf Stephan

Branch/Commit: u/rws/expressiontreewalker_fails_on_some_functions @ d2afc44

Issue created by migration from https://trac.sagemath.org/ticket/19464

@rwst rwst added this to the sage-6.10 milestone Oct 24, 2015
@rwst

This comment has been minimized.

@rwst

This comment has been minimized.

@rwst

This comment has been minimized.

@rwst
Copy link
Contributor Author

rwst commented Apr 3, 2016

@rwst
Copy link
Contributor Author

rwst commented Apr 3, 2016

comment:5

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

@rwst
Copy link
Contributor Author

rwst commented Apr 3, 2016

Commit: 4198c28

@rwst
Copy link
Contributor Author

rwst commented Apr 3, 2016

Author: Ralf Stephan

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 3, 2016

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

d2afc4419464: fix bug

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 3, 2016

Changed commit from 4198c28 to d2afc44

@videlec
Copy link
Contributor

videlec commented Apr 10, 2016

comment:7

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

@rwst
Copy link
Contributor Author

rwst commented Apr 11, 2016

comment:8

OK, duplicate.

@rwst rwst removed this from the sage-6.10 milestone Apr 11, 2016
@vbraun vbraun closed this as completed Jun 12, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants