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.

Sign up for GitHub

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

using a private symbol to store the state inside DisplayForm. #617

Open
wants to merge 3 commits into
base: DisplayForm
Choose a base branch
from
Open

using a private symbol to store the state inside DisplayForm. #617

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
aca9b16
using a private symbol to store the state.
mmatera Nov 13, 2022
475ef26
lock and protect the variable
mmatera Nov 13, 2022
8fd073e
fix name
mmatera Nov 13, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 30 additions & 5 deletions mathics/builtin/forms/output.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
from mathics.builtin.box.layout import GridBox, RowBox, to_boxes
from mathics.builtin.comparison import expr_min
from mathics.builtin.forms.base import FormBaseClass
from mathics.builtin.makeboxes import MakeBoxes, number_form
from mathics.builtin.makeboxes import MakeBoxes, SYMBOL_FORMATBOXES, number_form
from mathics.builtin.tensors import get_dimensions

from mathics.core.atoms import (
Expand All @@ -30,6 +30,7 @@
StringFromPython,
)

from mathics.core.attributes import A_LOCKED, A_PROTECTED
from mathics.core.expression import Expression, BoxError
from mathics.core.formatter import format_element
from mathics.core.list import ListExpression
Expand Down Expand Up @@ -188,12 +189,16 @@ def eval_makeboxes(self, expr, f, evaluation):
# MakeBoxes is formatting an expression inside a DisplayForm.
# Hopefully, this is temporal and it is not going to be
# needed after the Format/MakeBoxes refactor.
previous_df, evaluation.in_display_form = evaluation.in_display_form, True

old_value_in_display_form = SYMBOL_FORMATBOXES.evaluate(evaluation)
evaluation.definitions.set_ownvalue(SYMBOL_FORMATBOXES.name, SymbolTrue)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Notice that here we can not use the higher level way

Expression(SymbolSet, SYMBOL_NO_FORMATBOXES, SymbolTrue).evaluate(evaluation)

because the attributes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The drawbacks of this approach are that the access to the attribute is slower, and the code to set the context is larger and weirder than just setting an attribute.

try:
result = MakeBoxes(expr, f).evaluate(evaluation)
except:
pass
evaluation.in_display_form = previous_df
except Exception:
result = None
evaluation.definitions.set_ownvalue(
SYMBOL_FORMATBOXES.name, old_value_in_display_form
)
return result


Expand Down Expand Up @@ -1124,3 +1129,23 @@ def eval_makeboxes_matrix(self, table, f, evaluation, options):
return RowBox(String("("), result, String(")"))

return result


# Custom private symbol that helps DisplayForm to control if
# MakeBoxes must process BoxElements as expressions, or keep as they are.
class PrivateSymbolThatControlsTheStateOfDisplayForm(Builtin):
"""
<dl>
<dt>'PrivateSymbolThatControlsTheStateOfDisplayForm'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small thing - please indent nested <dt>.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rocky, the question is, do you think that this is a better approach?

Copy link
Member

@rocky rocky Nov 13, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mmatera good question. I have to think about it. What I'd be looking for is clues in the WMA code.

Think of this like the way a physicist would. We have two hypotheses for how this might have been coded in WMA. Is there a test that distinguishes these? Suppose we hypothesize that WMA follows the WMA way to do this, what test could we come up with that would distinguish the implementations? (Likewise for the other way to code)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ja, it is a curious request! Because it is exactly as this physicist is trying to distinguish what is the implementation in WMA :)

Look at this, for example,

imagen

If I believe in this output, I have to assume that there is not a private hidden symbol but an internal variable inside the kernel.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok - I trust your conclusion, but I don't follow it. The output above doesn't indicate anything to me one way or another. Please break it down for a novice.

If it were implemented as a private hidden symbol what then would you expect to see?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably implement TracePrint so that we can check correspondence easier.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would expect an evaluation for the corresponding hidden symbol. This is an example in which internal variables are used:

imagen

(I just cut the first lines)

Notice the use of symbols in hidden contexts (BoxFormls, IntegrateDumpTestLimits, BoxFormopt). Tracing MakeBoxes[DisplayForm[...]]` I do not see such kind of symbols in hidden contexts.

<dd>If True, MakeBoxes keep Box elements as they are. Otherwise,\
tries to format them as if it were expressions.
</dl>
"""

attributes = A_LOCKED | A_PROTECTED
context = "System`Private`MakeBoxes`"
name = "PrivateSymbolThatControlsTheStateOfDisplayForm"
rules = {
"System`Private`MakeBoxes`PrivateSymbolThatControlsTheStateOfDisplayForm": "False",
}
summary_text = "control if boxes are reevaluated."
9 changes: 8 additions & 1 deletion mathics/builtin/makeboxes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
from mathics.core.symbols import (
Atom,
Symbol,
SymbolTrue,
)

from mathics.core.systemsymbols import (
Expand All @@ -46,6 +47,12 @@
)


# Afterwards, we can find a better name
SYMBOL_FORMATBOXES = Symbol(
"System`Private`MakeBoxes`PrivateSymbolThatControlsTheStateOfDisplayForm"
)


def int_to_s_exp(expr, n):
n = expr.get_int_value()
if n < 0:
Expand Down Expand Up @@ -396,7 +403,7 @@ def apply_general(self, expr, f, evaluation):
if isinstance(expr, BoxElementMixin):
# If we are inside a DisplayForm block,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this really just DisplayForm that could do this? There are no other forms that are like this. Or this isn't something Form creator might want to do?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By now, this is the only case. This could also be a way to implement InputForm/OutputForm and the other non-standard print forms but setting a different control symbol. In any case, I am not sure that this way is better than the one I proposed before.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and the other non-standard print forms

What makes a print form "non-standard"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MakeBoxes accepts StandardForm|TraditionalForm as the second parameter. In some way, when you evaluate

MakeBoxes[OutputForm[expr], StandardForm]

Makeboxes first applies the format rules associated with OutputForm to expr and its subexpressions. Then, the reference to OutputForm are lost in the expression, but in some way, MakeBoxes knows that has to apply the OutputForm makeboxes rules to the inner expressions. One way to simulate this behavior would be to set a state variable that remembers that we are using OutputForm rules. This is not the way in which I think we should implement this, but it is a possibility.

Copy link
Member

@rocky rocky Nov 13, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but in some way, MakeBoxes knows that has to apply the OutputForm makeboxes rules to the inner expressions.

https://reference.wolfram.com/language/ref/TraditionalForm.html says:

TraditionalForm inserts invisible TagBox and InterpretationBox constructs into the box form of output it generates, to allow unique interpretation.tionalForm

Does this provide a mechanism for how to do this? If so, possibly the same is could done for StandardForm (and is done but just not mentioned). Sometimes it happens that only in implementing the second thing of a kind the general mechanism, and the first is done in a more hacky way is not retrofitted.

# BoxElementMixin are not processed.
if evaluation.in_display_form:
if SYMBOL_FORMATBOXES.evaluate(evaluation) is SymbolTrue:
return expr
expr = expr.to_expression()
if isinstance(expr, Atom):
Expand Down
2 changes: 0 additions & 2 deletions mathics/core/evaluation.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,8 +257,6 @@ def __init__(
self.quiet_all = False
self.format = format

# See comment in mathics.builtin.makeboxes.DisplayForm
self.in_display_form = False
self.catch_interrupt = catch_interrupt
self.SymbolNull = SymbolNull

Expand Down