Improvements to tackle wrong expression outputs#605
Conversation
|
You can get quite fancy with an
Eventually, the limit will still kick in:
We could issue a warning via |
|
With warning:
|
mathics/core/expression.py
Outdated
| cost = sum(leaf.output_cost() for leaf in self.leaves) | ||
|
|
||
| if name == 'System`List': | ||
| return 2 + cost + len(self.leaves) # {a, b, c} |
There was a problem hiding this comment.
would it be better to compute the sum cost of leaves? Also, what about the size of ", " between leaves?
mathics/core/expression.py
Outdated
| elif name in _layout_boxes: | ||
| return cost | ||
| else: | ||
| return cost + 2 + self.head.output_cost() + len(self.leaves) # XYZ[a, b, c] |
There was a problem hiding this comment.
cost is the sum cost of leaves (I renamed it now). len(self.leaves) measures the size of ",", which could be one or two (it assumes for now). we could pass the separator size into make_boxes and then pass it on to output_cost, but it's bulky and then should we really measure it that way? If $OutputSizeLimit is understood as number of printable characters (without spaces and layout elements like fractions, parentheses, ...), then it makes sense to always measure 1 for ",".
There was a problem hiding this comment.
In that case I guess technically it should be max(len(self.leaves) - 1, 0).
|
It seems like MMA kernels don't even observe |
|
So our approach is more efficient! Good to know |
mathics/core/expression.py
Outdated
| total_cost = 2 + cost_of_leaves + separator_cost * n_separators # {a, b, c}, [a, b, c] | ||
|
|
||
| if name != 'System`List': | ||
| total_cost += self.head.output_cost() + separator_cost * n_separators |
There was a problem hiding this comment.
doesn't this count the separators twice now?
|
This will need to be rebased on a revert of 8e14668. |
rocky
force-pushed
the
master
branch
3 times, most recently
from
87662f3 to
52e46c7
Compare
rocky
force-pushed
the
master
branch
4 times, most recently
from
ab12a77 to
6525df7
Compare
rocky
force-pushed
the
master
branch
3 times, most recently
from
7a4d68a to
d41fae8
Compare
rocky
force-pushed
the
master
branch
2 times, most recently
from
96ec58b to
e8a5440
Compare
|
I made a rebase in #1371. @poke1024 if you have the time and the will, look at it. |
5 participants
see #602