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 -> sympy conversion #188

Merged
merged 56 commits into from
Aug 14, 2023
Merged

Mathml -> sympy conversion #188

merged 56 commits into from
Aug 14, 2023

Conversation

kkaris
Copy link
Contributor

@kkaris kkaris commented Jul 6, 2023

This PR introduces the capability to parse sympy from mathml using the sbmlmath library.

The core functionality is added in mira.metamodel.io.mathml_to_expression using sbmlmath's SBMLMathMLParser.

mira/sources/askenet/petrinet.py is updated to handle cases when sympy expression strings are missing from the incoming askenet petrinet model json. sympy expressions still have priority over mathml when both are available.

Tests are added in the new file test_io.py.

todo

  • Write test where incoming askenet petrinet model json don't have sympy expression strings but mathml and compare parsed model to one parsed from an askenet petrinet model json with sympy expression strings.
  • Write test for the model api that tests the same as above Implicitly already tested in existing tests
  • sbmlmath requires py3.9+, mira tests run on 3.8 and 3.10. This can be resolves by either bumping python version for mira or by forcing the install on the docker that runs the service.

@kkaris
Copy link
Contributor Author

kkaris commented Jul 6, 2023

Tests are failing because there seems to be a problem with installing pandas. I haven't figured out why yet though, pandas is not a requirement for sbmlmath.

[...]
  Downloading pandas-0.25.1.tar.gz (12.6 MB)
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 12.6/12.6 MB 70.5 MB/s eta 0:00:00
  Preparing metadata (setup.py): started
  Preparing metadata (setup.py): finished with status 'done'
  Downloading pandas-0.25.0.tar.gz (12.6 MB)
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 12.6/12.6 MB 75.3 MB/s eta 0:00:00
  Preparing metadata (setup.py): started
  Preparing metadata (setup.py): finished with status 'done'
  Downloading pandas-0.24.2.tar.gz (11.8 MB)
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 11.8/11.8 MB 27.2 MB/s eta 0:00:00
  Preparing metadata (setup.py): started
  Preparing metadata (setup.py): finished with status 'error'
  error: subprocess-exited-with-error
  
  × python setup.py egg_info did not run successfully.
  │ exit code: 1
  ╰─> [13 lines of output]
      /home/runner/work/mira/mira/.tox/py/lib/python3.8/site-packages/setuptools/__init__.py:84: _DeprecatedInstaller: setuptools.installer and fetch_build_eggs are deprecated.
      !!
      
              ********************************************************************************
              Requirements should be satisfied by a PEP 517 installer.
              If you are using pip, you can try `pip install --use-pep517`.
              ********************************************************************************
      
      !!
        dist.fetch_build_eggs(dist.setup_requires)
      error in pandas setup command: 'install_requires' must be a string or list of strings containing valid project/version requirement specifiers; Expected end or semicolon (after version specifier)
          pytz >= 2011k
               ~~~~~~~^
      [end of output]
  
  note: This error originates from a subprocess, and is likely not a problem with pip.
error: metadata-generation-failed

× Encountered error while generating package metadata.
╰─> See above for output.

note: This is an issue with the package mentioned above, not pip.
hint: See above for details.

=================================== log end ====================================
___________________________________ summary ____________________________________
ERROR:   py: InvocationError for command /home/runner/work/mira/mira/.tox/py/bin/python -m pip install --exists-action w '/home/runner/work/mira/mira/.tox/.tmp/package/1/mira-0.1.0.tar.gz[tests,web]' (exited with code 1)
Error: Process completed with exit code 1.

@kkaris kkaris force-pushed the mathml-sympy branch 3 times, most recently from 30cf70c to cad7684 Compare July 12, 2023 20:05
@codecov-commenter
Copy link

codecov-commenter commented Jul 12, 2023

Codecov Report

Merging #188 (9338449) into main (55d6e33) will increase coverage by 0.22%.
The diff coverage is 89.24%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@            Coverage Diff             @@
##             main     #188      +/-   ##
==========================================
+ Coverage   77.37%   77.59%   +0.22%     
==========================================
  Files          66       67       +1     
  Lines        5277     5369      +92     
==========================================
+ Hits         4083     4166      +83     
- Misses       1194     1203       +9     
Impacted Files Coverage Δ
tests/__init__.py 79.03% <79.03%> (ø)
mira/sources/askenet/petrinet.py 89.74% <86.20%> (-1.57%) ⬇️
mira/metamodel/io.py 81.48% <100.00%> (+4.20%) ⬆️
mira/metamodel/units.py 100.00% <100.00%> (ø)
tests/test_metamodel.py 100.00% <100.00%> (ø)
tests/test_model_api.py 98.97% <100.00%> (+2.41%) ⬆️

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

setup.cfg Outdated Show resolved Hide resolved
@bgyori bgyori merged commit 50df150 into gyorilab:main Aug 14, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants