Improving MathML transpiling

[jonc125]

Description

Moved the transpiler into parser.py and made it use lxml (elementtree API) rather than DOM. This means we don't need to serialise MathML to string & re-parse to convert to sympy.

Motivation and Context

Fixes #51. Should also help with #208.

Types of changes

• [x] Bug fix (non-breaking change which fixes an issue) - at least, it's non-breaking provided users haven't accessed transpiler.py directly!

Checklist:

• [x] I have updated all documentation necessary.
• [x] I have checked spelling in (new) comments.

Testing

• [x] Testing is done automatically and codecov shows test coverage

[codecov[bot]]

Codecov Report

Merging #284 into develop will increase coverage by 1.01%. The diff coverage is 100.00%.

@@             Coverage Diff             @@
##           develop     #284      +/-   ##
===========================================
+ Coverage    97.57%   98.58%   +1.01%     
===========================================
  Files            7        6       -1     
  Lines         1359     1346      -13     
  Branches       295      293       -2     
===========================================
+ Hits          1326     1327       +1     
+ Misses          15        7       -8     
+ Partials        18       12       -6     

Impacted Files	Coverage Δ
cellmlmanip/model.py	99.78% <100.00%> (ø)
cellmlmanip/parser.py	97.39% <100.00%> (+1.53%)	:arrow_up:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update f7d43a7...fcef332. Read the comment docs.

[MichaelClerx]

Also (but maybe less pressing), I found that if you use an assert instead of raising a value error it plays nicely with the code coverage, so e.g. assert len(result) == 2, 'Need exactly 2 children for <piece>' which f triggered gives: AssertionError: Need exactly 2 children for <piece> as opposed to

Asserts should never go from cellmlmanip to a cellmlmanip user though. They're sanity checks, not part of the the public API.

[MichaelClerx]

Also, coverage and style checks are there to help. So adapting the code for no other reason than to make them pass is a terrible idea :D

[MichaelClerx]

I even try and write commit messages like "improved code style" rather than "fixed flake8", just to remind myself that flake8 is a tool, not the goal :-| (sorry for being so boring)

[MauriceHendrix]

Also, coverage and style checks are there to help. So adapting the code for no other reason than to make them pass is a terrible idea :D

Sure, but really in my opinion an assert is a better fit for something that should be true but where you think it feels like something that might go wrong in the future, even if there is no way of making it fail right now. I would say raise is more appropriate for errors you are actively expecting and handling.

[MichaelClerx]

This one's asking how many children in a <piece> though, that's determined by the CellML file you give it as input, isn't it? Might be that this gets caught by an earlier validation, which would make it unreachable and in that case I fully agree with you :D

[MauriceHendrix]

This one's asking how many children in a <piece> though, that's determined by the CellML file you give it as input, isn't it? Might be that this gets caught by an earlier validation, which would make it unreachable and in that case I fully agree with you :D

Well if it doesn't then it would need a test contrived cellml file as case no?