Use sbmlmath package for sympification of SBML math constructs#2681
Use sbmlmath package for sympification of SBML math constructs#2681dweindl merged 32 commits intoAMICI-dev:developfrom
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #2681 +/- ##
===========================================
- Coverage 77.83% 77.75% -0.08%
===========================================
Files 332 332
Lines 23001 23031 +30
Branches 1480 1480
===========================================
+ Hits 17902 17907 +5
- Misses 5088 5113 +25
Partials 11 11
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Directly parse the SBML MathML constructs without going through SBML L3 formula strings are sympy.sympify due to various issues (AMICI-dev#2146).
1fd6c54 to
cd55a97
Compare
65fb47e to
2a635ff
Compare
b24f490 to
ac418ba
Compare
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
| f"{err}." | ||
| except (sp.SympifyError, TypeError, ZeroDivisionError) as err: | ||
| raise SBMLException( | ||
| f'{ele_name} "{var_or_math}" ' |
There was a problem hiding this comment.
this will never be triggered from ele_name == "SymPy expression", right? Should the try claus also include "isinstance(var_or_math, sp.Basic)"?
There was a problem hiding this comment.
this will never be triggered from ele_name == "SymPy expression", right?
right. that ele_name can be removed.
Should the try claus also include "isinstance(var_or_math, sp.Basic)"?
I don't think so. I don't recall errors during .subs()
| ), | ||
| ) | ||
| try: | ||
| expr = self._mathml_parser.parse_str(mathml) |
There was a problem hiding this comment.
same as above, make this one big try clause?
There was a problem hiding this comment.
I don't think .subs() will raise, I'd leave it out of there.
Directly parse the SBML MathML constructs without going through SBML L3 formula strings and
sympy.sympify()due to various issues (#2146).