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

Pretty print #1162

Draft
wants to merge 21 commits into
base: master
Choose a base branch
from
Draft

Pretty print #1162

wants to merge 21 commits into from

Conversation

mmatera
Copy link
Contributor

@mmatera mmatera commented Nov 10, 2024

This is a (partial) reimplementation of #1155, based on sympy.printing.pretty.stringpict.prettyForm objects. #1155 was written following the same idea of that class, but in a way that supports "standard" string operations instead of "prettyPrint". With this I mean for example, that text1_TextBlock + text2_TextBlock returns the concatenation of both text blocks.

In prettyForm objects, text1_TextBlock + text2_TextBlock produces a block with a sign + or - depending on the second argument represents a positive or a negative quantity. I find it weird, but it seems it works for Sympy.

Issues with the current implementation of (Sympy) PrettyForm

sympy.printing.pretty.stringpict.prettyForm also has other problems, like bugs in the implementation of prettyForm.root, and a weird interface to concatenate text blocks. Also, some features I would like to have for these objects are still not implemented, like handling text aligning.

All of this can be improved by asking Sympy's guys to consider implementing some fixes, something that I have eventually planned to do.

Why not building over the higher level PrettyPrinter?

Regarding using PrettyPrinter instead of this lower-level class is motivated in that PrettyPrinter is designed to work with Sympy objects. The equivalent of the "head" of a Mathics expression in Sympy is the class of the object. So, to support Mathics objects by subclassing PrettyPrinter essentially would mean to define methods _print_Expression or _print_Atom, and do what I have already done in mathics.format.prettyprint: define a dispatch table to associate heads and Mathics types to formatter functions.

Another possibility would be to implement a special converter from Mathics expressions to Sympy, which would require duplicating the conversion function. Also, it would necessarily be a less flexible approach, and the representation of certain WL objects would be less precise or even more hacky.

@rocky
Copy link
Member

rocky commented Nov 10, 2024

This is a (partial) reimplementation of #1155, based on sympy.printing.pretty.stringpict.prettyForm objects. #1155 was written following the same idea of that class, but in a way that supports "standard" string operations instead of "prettyPrint". With this I mean for example, that text1_TextBlock + text2_TextBlock returns the concatenation of both text blocks.

In prettyForm objects, text1_TextBlock + text2_TextBlock produces a block with a sign + or - depending on the second argument represents a positive or a negative quantity. I find it weird, but it seems it works for Sympy.

Issues with the current implementation of (Sympy) PrettyForm

sympy.printing.pretty.stringpict.prettyForm also has other problems, like bugs in the implementation of prettyForm.root, and a weird interface to concatenate text blocks. Also, some features I would like to have for these objects are still not implemented, like handling text aligning.

All of this can be improved by asking Sympy's guys to consider implementing some fixes, something that I have eventually planned to do.

Why not building over the higher level PrettyPrinter?

Regarding using PrettyPrinter instead of this lower-level class is motivated in that PrettyPrinter is designed to work with Sympy objects. The equivalent of the "head" of a Mathics expression in Sympy is the class of the object. So, to support Mathics objects by subclassing PrettyPrinter essentially would mean to define methods _print_Expression or _print_Atom, and do what I have already done in mathics.format.prettyprint: define a dispatch table to associate heads and Mathics types to formatter functions.

Another possibility would be to implement a special converter from Mathics expressions to Sympy, which would require duplicating the conversion function. Also, it would necessarily be a less flexible approach, and the representation of certain WL objects would be less precise or even more hacky.

Briefly looking at the code, I have a gut feeling that something is amiss. It doesn't fully follow the kind of tree parsing pattern that I often have seen in code. And instead, the code keeps stuff like String("/") which is reminiscent of boxing code from Mathics that feels a little too rigid and should be more table-based.

An approach that can build on and contribute back to SymPy feels like a better approach overall, but this is more of a gut feeling that digging into the code deeply right now with more concrete suggestions.

Let's defer merging this until we have boxing under better control.

@mmatera
Copy link
Contributor Author

mmatera commented Nov 13, 2024

@rocky, I put a PR in Sympy related to this sympy/sympy#27257
If it get's merged, then I can reformulate this using the sympy.printing.pretty.prettyForm class directly.

@rocky
Copy link
Member

rocky commented Nov 14, 2024

@rocky, I put a PR in Sympy related to this sympy/sympy#27257 If it get's merged, then I can reformulate this using the sympy.printing.pretty.prettyForm class directly.

I am very glad to see some sort of collaboration (as opposed to reinvention) here. Thanks for engaging with SymPy.

Let's wait on this to see where things go.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants