Conversation
Contributor
|
The problem here is that both ways are broken: mglyph does not work anymore because MathML standard changed. The mtext way does not work because the way in which we produce the svg images is also based on this outdated mglyph behaviour. However, the mtext version allows us to patch this with the pymathics modules we are developing, while the mglyph doesn't. I overwrite this method in such classes, so I think that this change does not harm the pymathics mechanism to fix this. Go ahead. |
mmatera
requested changes
Mar 29, 2021
Contributor
mmatera
left a comment
There was a problem hiding this comment.
leave the line as a comment.
5effca2 to
1ff00b1
Compare
rocky
added a commit
that referenced
this pull request
Mar 29, 2021
mglyph -> mtext broke some images
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
@mmatera I used
git bisectand tracked the graphics misbehaving to 616dbba and specificlly the one-line change here.What does this change make possible? If we can't figure out something that accomodates both concerns, maybe we can add a setting so both ways are possible.
BTW This PR is the kind of thing I was mentioning about large PR's that are hard to follow.
The title of #1173 is "adds FileNames, a symbol required in the FeynCalc package" and I don't see how this change which is about graphics has anything to do with ByteArray or FileNames. Even the commit message "fix write" isn't suggestive.