-
-
Notifications
You must be signed in to change notification settings - Fork 64
Misc tweaks... #1599
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
Merged
Merged
Misc tweaks... #1599
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Empty file.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe the right place for
outputform.pywould be insideeval/makeboxes, likeinputform.py.mathics.formatcontains modules to convert (Boxed) expressions into strings in different representations (plane text, latex, mathml). OutputForm (and InputForm) does something similar, but on (norma) expressions.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At some level, it does not matter that much other than "Form" handling and rendering are different concepts and levels; so
mathics.formatis definitely the wrong place.Personally, I think of
mathics.evalhaving code to implement the primitives of built-in functions.Of course, there are primitives for boxing. So, sure, this fits into this category. However...
Boxing is considered its own phase. And while it is a kind of evaluation too, it is not the kind of primitive used for addition and multiplication, which must be implemented in Python, SymPy, etc.
In theory, Boxing could be written purely as Mathics3 or WMA code. And there is enough of it that, in my opinion, we'd be better off just putting that in its own place.
But again, other than it not being part of
mathics.format, I don't have a strong feeling like this. Aesthetically, I think it is just better put inmathics.form.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At this point, I agree.
OK, but we have a specific submodule for Box evaluations
If in principle that is true, it is also true that we could implement in WL most of the code inside "eval". However, would be unnecesarily complicated and very slow.
I do not have strong feelings about this either, just an argument: from the point of view of code organization, it appears that we would have a lot of space for the different steps of formatting. On the other hand, the complexity inside would not be comparable with the one in
core,builtinoreval.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personally, I'd like the boxing and form stuff put somewhere else, since I think of eval as more of the kind of stuff that a compiler would work on.
So, again this is the kind of thing, in my view of the code organization that should distinguish eval from other parts. Stuff that really has to be done in Python (Sympy, numpy, etc.) is definitely in eval. Stuff that could easily be done in WMA, is somewhere else; especially if it is a well-defined "phase". There may be gray areas in the middle.
Analogy: think of "eval" as "physics". But there is "chemistry" and "organic chemistry" as more like "boxing" and "forms". While this could be put under the category of "physics", typically we don't do it in that way
If you don't have a strong feeling, would you mind catering again to my idiosyncracies, and let's put this in either "mathics.form" or "mathics.box" or have two modules?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rocky, OK, let's do as you proposed here and see how this evolve.