Document `is_expression_type` and other PyomoObject methods

[carldlaird]

Objects within the expression system are required to implement is_expression_type. It appears that this is used to indicate if an object is a leaf in the expression tree, or something that is intended to have arguments (children). For example, NumericValue (which is definitely part of the expression system) returns False for this method. I believe this should be renamed to "has_args" or "has_children" or "is_leaf_node" to be more clear.

@whart222 Am I missing other uses for this method, or is my understanding correct?

[michaelbynum]

Not that this is helpful, but this was my understanding of the method too.

[jsiirola]

I believe that your understanding is effectively correct, but that the original distinction was slightly different: foo.is_expression_type() == True was supposed to be equivalent to isinstance(ExpressionBase, foo) == True. It was included to enable duck typing (e.g., for the Expression component) and because the method call was faster than a call to isinstance (especially for non-expression types).

That said, I believe that all derivatives of ExpressionBase are "non-leaf" nodes in any expression tree, so @carldlaird's interpretation is correct.

[whart222]

FWIW, the reason not to use is_leaf_node (or something similar) is that there has been some debate about what constitutes a "leaf". Specifically, Expression components have been discussed as leaf objects by various team members. Hence, this naming semantics was used.

[jsiirola]

@carldlaird: does something like is_operator_type better describe the behavior of is_expression_type? That is, it is True if the node has args?

That said, my description appears to support your suggestion of has_args or just hasattr(expr, ‘args’); but for reasons I cannot articulate, those solutions do “feel right”. Maybe it is that since the method is used to determine the correct way to process a node, some part of me feels the method should start with “is_”?

[carldlaird]

I have less of an issue with the name now that I understand it more. I believe that is_expression_type() is really indicating if this class supports (or ducktypes) the "interface" defined by ExpressionBase (i.e. does it contain the requisite methods). However, in cases where it is really being used to determine if args exist, then I feel the 'hasattr' approach is better. I assume this is what is done when other code expects an iterator, for example.

[mrmundt]

@jsiirola - Status update on this?