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

MathML presentation printing for Range, Min, Max, exp, true, false, None #16126

Merged
merged 3 commits into from Mar 9, 2019

Conversation

oscargus
Copy link
Contributor

@oscargus oscargus commented Mar 1, 2019

References to other Issues or PRs

Brief description of what is fixed or changed

Added MathML presentation printing for Range, Min, Max, exp, true, false, None

Before:
before161xx

After:
after161xx

Other comments

Release Notes

  • printing
    • Added MathML presentation printing for Range, Min, Max, exp, true, false, None

@sympy-bot
Copy link

sympy-bot commented Mar 1, 2019

Hi, I am the SymPy bot (v142). I'm here to help you write a release notes entry. Please read the guide on how to write release notes.

Your release notes are in good order.

Here is what the release notes will look like:

  • printing
    • Added MathML presentation printing for Range, Min, Max, exp, true, false, None (#16126 by @oscargus)

This will be added to https://github.com/sympy/sympy/wiki/Release-Notes-for-1.4.

Note: This comment will be updated with the latest check if you edit the pull request. You need to reload the page to see it.

Click here to see the pull request description that was parsed.

<!-- Your title above should be a short description of what
was changed. Do not include the issue number in the title. -->

#### References to other Issues or PRs
<!-- If this pull request fixes an issue, write "Fixes #NNNN" in that exact
format, e.g. "Fixes #1234". See
https://github.com/blog/1506-closing-issues-via-pull-requests . Please also
write a comment on that issue linking back to this pull request once it is
open. -->


#### Brief description of what is fixed or changed
Added MathML presentation printing for `Range`, `Min`, `Max`, `exp`, `true`, `false`, `None`

Before:
![before161xx](https://user-images.githubusercontent.com/8114497/53668203-844c2880-3c73-11e9-910e-a60835a814ef.PNG)

After:
![after161xx](https://user-images.githubusercontent.com/8114497/53668216-8b733680-3c73-11e9-976d-c07d7db552ed.PNG)


#### Other comments


#### Release Notes

<!-- Write the release notes for this release below. See
https://github.com/sympy/sympy/wiki/Writing-Release-Notes for more information
on how to write release notes. The bot will check your release notes
automatically to see if they are formatted correctly. -->

<!-- BEGIN RELEASE NOTES -->
* printing
    * Added MathML presentation printing for Range, Min, Max, exp, true, false, None
<!-- END RELEASE NOTES -->

Update

The release notes on the wiki have been updated.

@asmeurer
Copy link
Member

asmeurer commented Mar 7, 2019

Why do you use blackboard font for e?

@asmeurer
Copy link
Member

asmeurer commented Mar 7, 2019

Also is the last entry in the list a typo? It shows -oo instead of -∞.

assert mpp.doprint(Range(30, 1, -1)) == '<mfenced close="}" open="{"><mn>30</mn><mn>29</mn><mi>&#8230;</mi><mn>2</mn></mfenced>'
assert mpp.doprint(Range(0, oo, 2)) == '<mfenced close="}" open="{"><mn>0</mn><mn>2</mn><mi>&#8230;</mi><mi>&#x221E;</mi></mfenced>'
assert mpp.doprint(Range(oo, -2, -2)) == '<mfenced close="}" open="{"><mi>&#x221E;</mi><mi>&#8230;</mi><mn>2</mn><mn>0</mn></mfenced>'
assert mpp.doprint(Range(-2, -oo, -1)) == '<mfenced close="}" open="{"><mn>-2</mn><mn>-3</mn><mi>&#8230;</mi><mn>-oo</mn></mfenced>'
Copy link
Member

Choose a reason for hiding this comment

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

I guess it wasn't a typo. This should be fixed.

@asmeurer
Copy link
Member

asmeurer commented Mar 7, 2019

I guess exp is using exponentiale. According to https://www.w3.org/TR/MathML2/chapter4.html#contm.exponentiale it renders using a normal e, but I guess whatever you made your table with uses a blackboard font?

Also according to https://www.w3.org/TR/MathML2/chapter4.html#contm.exp there is a exp which should be used for the exponential function.

@asmeurer
Copy link
Member

asmeurer commented Mar 7, 2019

That is assuming I'm not mixing up presentation and content MathML. I'm not clear how those work in the printer, or which the above document refers to.

@oscargus
Copy link
Contributor Author

oscargus commented Mar 8, 2019

Yes, exponential e is used for exp as it was earlier used for printing a single e. Not sure why Firefox and/or the font maps it to blackboard. My rationale was that if there is a special character defined for it, that is the one to be used (rather than discussing if it should be italic or non-italic etc).

I'll check the documentation more carefully, but my guess is that exp is content. Either way we can choose how we want it rendered, but for consistency I think something with e to the power is preferred.

Will get back on this later on.

@oscargus
Copy link
Contributor Author

oscargus commented Mar 8, 2019

Yes, the printing of -oo is still in the rather long list of functions to fix. Not sure why it behaves as it does. So the behaviour here is "as expected" given the current code base. (Same thing with complex infinity.)

@oscargus
Copy link
Contributor Author

oscargus commented Mar 8, 2019

I've found the issue with -oo and will fix...

def _print_Negative_Infinity(self, e):

@asmeurer
Copy link
Member

asmeurer commented Mar 8, 2019

I don't know very much about MathML so I'll defer to you on how to best print it. I though we supported both content and presentation MathML, though.

If the blackboard e is coming from the renderer then we shouldn't worry about it.

@oscargus
Copy link
Contributor Author

oscargus commented Mar 9, 2019

Well, there are printers for both Content and Presentation MathML. I've focused on Presentation since it is more "fun" to work with (easy to see the effect of changes etc). Also, Content MathML probably works in more cases already as it much more relies on the actual function names and not on specific printing of it (for example, the range printing was probably OK from a "content" perspective as it was, bad comparison since the content MathML looks different, but the idea hold, printing a function name and arguments is more often OK Content MathML). Finally, some things may not even be relevant/supported from a Content perspective.

So the conclusion is basically that, sure, it would be great if someone worked on the Content MathML as well. Not sure if we shouldn't merge Presentation MathML improvements because of that though.

@smichr smichr merged commit 8fc3b3a into sympy:master Mar 9, 2019
@oscargus oscargus deleted the mathmlconstantsminmaxrange branch March 9, 2019 13:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants