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

Add set operators printing for MathML content printer #17545

Merged
merged 3 commits into from
Dec 24, 2019

Conversation

sylee957
Copy link
Member

@sylee957 sylee957 commented Aug 29, 2019

References to other Issues or PRs

#17538

Brief description of what is fixed or changed

In #17538 I've mentioned mathml printing for set operators. I had implemented most of the set operators except for SymmetricDifference, which may not exist as not mentioned in the doc.

However, I see there are some problems with mathjax renders which have to be investigated.

Other comments

I'm not sure why this happens in mathjax.
Can anyone confirm that this is a correct markup?

It is the upstream issue that the parenthesis do not appear on some mathml content printing mathjax/MathJax#2184

<!DOCTYPE html>
<html>
<head>
  <meta charset="utf-8">
  <script>window.MathJax = { MathML: { extensions: ["mml3.js", "content-mathml.js"]}};</script>
<script type="text/javascript" async src="https://cdnjs.cloudflare.com/ajax/libs/mathjax/2.7.0/MathJax.js?config=MML_HTMLorMML"></script>
</head>
<body>
<math display="block">

<apply>
  <union/>
  <apply>
    <intersect/>
    <set><ci>a</ci></set>
    <set><ci>b</ci></set>
  </apply>
  <apply>
    <intersect/>
    <set><ci>c</ci></set>
    <set><ci>d</ci></set>
  </apply>
</apply>

</math>
</body>
</html>

image

For example, plus and times work well in similar syntax, but only some set operators are not properly parenthesized

<apply>
    <times/>
    <apply><plus/><ci> a </ci><ci> b </ci></apply>
    <apply><plus/><ci> c </ci><ci> d </ci></apply>
</apply>

image

Release Notes

  • printing
    • Added support for FiniteSet printing in mathml content markup.
    • Added support for Union, Intersection, Complement, and ProductSet printing for mathml content markup.

@sympy-bot
Copy link

sympy-bot commented Aug 29, 2019

Hi, I am the SymPy bot (v149). 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 support for FiniteSet printing in mathml content markup. (#17545 by @sylee957)

    • Added support for Union, Intersection, Complement, and ProductSet printing for mathml content markup. (#17545 by @sylee957)

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

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

#17538

#### Brief description of what is fixed or changed

In #17538 I've mentioned mathml printing for set operators. I had implemented most of the set operators except for `SymmetricDifference`, which may not exist as not mentioned in the [doc](https://www.w3.org/TR/MathML2/chapter4.html#contm.sets).

However, I see there are some problems with mathjax renders which have to be investigated.

#### Other comments

<!-- 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. -->

~~I'm not sure why this happens in mathjax.
Can anyone confirm that this is a correct markup?~~

It is the upstream issue that the parenthesis do not appear on some mathml content printing https://github.com/mathjax/MathJax/pull/2184

```HTML
<!DOCTYPE html>
<html>
<head>
  <meta charset="utf-8">
  <script>window.MathJax = { MathML: { extensions: ["mml3.js", "content-mathml.js"]}};</script>
<script type="text/javascript" async src="https://cdnjs.cloudflare.com/ajax/libs/mathjax/2.7.0/MathJax.js?config=MML_HTMLorMML"></script>
</head>
<body>
<math display="block">

<apply>
  <union/>
  <apply>
    <intersect/>
    <set><ci>a</ci></set>
    <set><ci>b</ci></set>
  </apply>
  <apply>
    <intersect/>
    <set><ci>c</ci></set>
    <set><ci>d</ci></set>
  </apply>
</apply>

</math>
</body>
</html>
```
![image](https://user-images.githubusercontent.com/34944973/63970766-ee893d00-cadf-11e9-908c-b62e90eab5c5.png)

For example, plus and times work well in similar syntax, but only some set operators are not properly parenthesized
```
<apply>
    <times/>
    <apply><plus/><ci> a </ci><ci> b </ci></apply>
    <apply><plus/><ci> c </ci><ci> d </ci></apply>
</apply>
```
![image](https://user-images.githubusercontent.com/34944973/63971510-981cfe00-cae1-11e9-96cb-6d7fa11c866d.png)

#### 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 support for `FiniteSet` printing in mathml content markup.
  - Added support for `Union`, `Intersection`, `Complement`, and `ProductSet` printing for mathml content markup.

<!-- END RELEASE NOTES -->

Update

The release notes on the wiki have been updated.

@codecov
Copy link

codecov bot commented Aug 29, 2019

Codecov Report

Merging #17545 into master will increase coverage by 0.034%.
The diff coverage is 100%.

@@              Coverage Diff              @@
##            master    #17545       +/-   ##
=============================================
+ Coverage   74.711%   74.745%   +0.034%     
=============================================
  Files          633       633               
  Lines       164617    164634       +17     
  Branches     38657     38660        +3     
=============================================
+ Hits        122987    123056       +69     
+ Misses       36235     36189       -46     
+ Partials      5395      5389        -6

@oscarbenjamin oscarbenjamin added the PR: author's turn The PR has been reviewed and the author needs to submit more changes. label Oct 4, 2019
@czgdp1807
Copy link
Member

@sylee957 Any updates on this?

@sylee957 sylee957 marked this pull request as ready for review November 5, 2019 18:30
@sylee957
Copy link
Member Author

sylee957 commented Nov 5, 2019

I’d mark this for review, since the only thing that is needed is verifying how correct the content printing is.

@sylee957 sylee957 removed the PR: author's turn The PR has been reviewed and the author needs to submit more changes. label Nov 5, 2019
@sylee957 sylee957 changed the title [WIP] Add set operators printing for MathML content printer Add set operators printing for MathML content printer Nov 5, 2019
@oscarbenjamin
Copy link
Contributor

I tried a few online mathml viewers with some of the examples here and they just show the letters with no mathematical symbols.

@sylee957
Copy link
Member Author

Which online viewer are you referring to?

assert mathml(ProductSet(C1, C2)) == \
'<apply><cartesianproduct/><apply><setdiff/><set><ci>a</ci></set>' \
'<set><ci>b</ci></set></apply><apply><setdiff/><set>' \
'<ci>c</ci></set><set><ci>d</ci></set></apply></apply>'
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this example have brackets? It comes out as roughly a - b x c - d but I think it should be (a - b) x (c - d).

@oscarbenjamin
Copy link
Contributor

I tried e.g. this one: https://www-archive.mozilla.org/projects/mathml/demo/tester

That doesn't seem to work too well though. I just checked what they look like in a local html file with chrome and they come out looking nice.

@sylee957
Copy link
Member Author

sylee957 commented Dec 18, 2019

Okay, I can explain the reasons behind

  1. MathML content markup is actually a markup for symbolic representation, like how SymPy represents things as Mul, Pow, rather than specifying every objects as brackets, subscripts, superscripts. However, I don't think that every mathml viewers have implemented this yet.

  2. The only place where the content markup had been implemented is mathjax, as far as I know. And the reason why my example can work in your local browser is that there is a html script that automatically downloads the mathjax. If you right-click on the math, you can see if it uses the mathjax or not.

  3. The way how mathjax processes the content markup is by using mtop library, which is a self-contained javascript library that converts the content markup to a respective presentation markup like the way how SymPy prints.

  4. However, I have already found a lot of fragile implementations in mtop as I have referenced the issue, and that's the reason that some brackets are not correctly formatted.

So I think that the mathjax is too buggy right now to test this out.
I'm not sure that other javascript libraries like KaTeX, or any other CAS have implemented the mathml content markup properly,
But if there is none, then we can only check the way how the standard is defined https://www.w3.org/TR/MathML3/chapter4.html. And I think that the apply markup should correctly render the brackets.

@oscarbenjamin
Copy link
Contributor

Okay I'll merge this now.

The standard says

In MathML, the apply element is used to build an expression tree that represents the application a
function or operator to its arguments. The resulting tree corresponds to a complete mathematical
expression. Roughly speaking, this means a piece of mathematics that could be surrounded by
parentheses or "logical brackets" without changing its meaning.

From that I don't know whether the display form should have brackets or not but this can always be fixed later if we find some other way to test it.

@oscarbenjamin oscarbenjamin merged commit 41ef7c5 into sympy:master Dec 24, 2019
@sylee957
Copy link
Member Author

I think that this is causing the failing in the master. I don’t know what was going wrong from here, but will figure it out

@sylee957 sylee957 deleted the fix_mathml_sets branch January 11, 2020 20:32
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