[request for comment] should Tuple hold non-Basic args?

9 views
Skip to first unread message

smichr

unread,
May 29, 2012, 2:46:05 AM5/29/12
to sy...@googlegroups.com
In  https://github.com/sympy/sympy/pull/1314 the behavior of Tuple is being changed wrt how the method 'has' applies to it. If an arg to 'has' is non-Basic, e.g. a bool or Function('f'), then the 'has' result will be False. The rationale is that has checks things that should only contain Basic objects. The problem is that presently Tuple will hold any sympifiable object. So Tuple(Function('f')).has(f) will return False after the changes but currently returns True. (The case of Tuple(None).has(None) raises an error in master because None does not have the is_Add attribute. In the proposed changes, it will fail with "AttributeError: 'NoneType' object has no attribute '_has_matcher'."

Eventually I think bool will become a Bool object in SymPy and then has will work with Bool (if it's a Basic object); but functions like 'f' are not Basic but they are a sympy object and although we don't currently put them in Tuple anywhere, I don't think it would be unreasonable to have them there and then I think has should work with them.

So is there a good reason to either a) not have f be Basic or b) not let Tuple hold functions like f (and not discriminate against them in a has query)?

/c

krastano...@gmail.com

unread,
May 29, 2012, 6:31:35 AM5/29/12
to sy...@googlegroups.com
>
> So is there a good reason to either a) not have f be Basic or b) not let
> Tuple hold functions like f (and not discriminate against them in a has
> query)?
>

a) To me it seems strange that f is not Basic (Lambda is an Expr for
instance). Moreover if we want the recursively evaluated Expr from
another thread here on the mailing list, we need f to be Expr

b) There is already "has not supported for non-Basic without
_has_matcher". So we have defined the interface we want. On the other
hand, it is not pythonic (as far as I understand it, I might be wrong)
to force the user to subclass Basic in order to work with sympy.

More generally, I do not understand why we want everything to be
Basic. The only argument that I hear is "in order to rebuild the
expressions". But rebuilding uses `type(object)(object.args)`, Why
should we force the use of Basic in addition to that. If an object has
.args we rebuild it as shown. If not we either raise an error or copy
it (copying is much better as it allows having strings in args - just
check every class that has a 'name' attribute to see how useful this
would be).

smichr

unread,
May 29, 2012, 11:55:56 AM5/29/12
to sy...@googlegroups.com
a) To me it seems strange that f is not Basic (Lambda is an Expr for
instance). Moreover if we want the recursively evaluated Expr from
another thread here on the mailing list, we need f to be Expr


It actually (in the branch in question, at least) is a BasicType which seems wrong.

>>> isinstance(Add, BasicType)
True
>>> isinstance(x+y, BasicType)
False
>>> isinstance(Function, BasicType)
True
>>> isinstance(Function('f'), BasicType)
True

It seems that are particular instance of Function (or undefinedFunction) should not also be considered a BasicType just as a particular Add is not.

/c

krastano...@gmail.com

unread,
May 29, 2012, 12:04:57 PM5/29/12
to sy...@googlegroups.com
Actually this is just a bit of dark magic (that will presumably will
be exchanged for a more sane api): Function('f') is NOT an instance of
Function, rather it is a subclass. In this design it does make sense
for it to be BasicType, it is just that I consider the overall idea
needlessly complicated.

Joachim Durchholz

unread,
May 29, 2012, 2:03:28 PM5/29/12
to sy...@googlegroups.com
Am 29.05.2012 08:46, schrieb smichr:
> In https://github.com/sympy/sympy/pull/1314 the behavior of Tuple is being
> changed wrt how the method 'has' applies to it. If an arg to 'has' is
> non-Basic, e.g. a bool or Function('f'), then the 'has' result will be
> False. The rationale is that has checks things that should only contain
> Basic objects.

In that case, 'has' should not pretend to know a valid answer and return
False.
Instead, it should assume that it cannot produce a valid answer and
throw an exception. Let the caller deal with the problem (which will
likely bubble up to some level that reports the problem and fails
execution - of course, if some intermediate software level built that
Tuple just for optimization, it can then catch the exception and try an
alternate strategy).

smichr

unread,
Jun 6, 2012, 1:06:27 AM6/6/12
to sy...@googlegroups.com
 

 
The good work of #1314 is stalled. We need a decision to be made: 

If we want to keep the existing ability of Tuple answer queries about containing UndefinedFunctions then #1331 can be committed. It is the same as #1314 except that  Tuple(Function('f'), True).has(True, Function('f')) will neither throw an error nor answer False. It also uses a library method to replace a piece of older code. Test results should post soon at https://github.com/sympy/sympy/pull/1331 .

If we eventually disallow Tuple to hold Function('f') and bool, that's a bridge that can be crossed later and has() modified accordingly, but I don't like the idea of changing has()'s behavior (and answering False for Tuple(Function('f')).has(Function('f'))) and would like to see #1331 be committed. Again, if we can just get a decision on this then we can commit the changes.

/c
Reply all
Reply to author
Forward
0 new messages