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

Version 0.2.2 #122

Merged
merged 12 commits into from
Mar 21, 2021
Merged

Version 0.2.2 #122

merged 12 commits into from
Mar 21, 2021

Conversation

jvanhoefer
Copy link
Member

No description provided.

jvanhoefer and others added 9 commits February 22, 2021 16:41
* add missing release notes

* minor changes
* add logo license; fixes #108

* add pypi badge

* invert python versions for readability
* .gitignore build folders

* test macos

* test windows

* fix cache

* fix typo

* add all output files to .gitignore
* fix notebooks

* add tests if SBML parts were created succesfully

* change to libsbml.LIBSBML_OPERATION_SUCCESS

* fix yanniks remarks

* try fixing hdf5

* try without hdf5

* try ubuntu 18.04

* try ubuntu-latest again

* simplify build

* test cache

* make build windows-compatible

Co-authored-by: dilpath <dilan.private+github@outlook.com>
Co-authored-by: yannikschaelte <yannik.schaelte@gmail.com>
* update swig

* add pydocstyle, min versions of flake8

* fix pydocstyle
* unfix AMICI

* fix notebooks

Co-authored-by: Yannik Schälte <31767307+yannikschaelte@users.noreply.github.com>
* add missing release notes (#111) (#112)

* add missing release notes

* minor changes

* version ++1

* minor typos in docs

* add release notes

* Update doc/release_notes.rst

Co-authored-by: Yannik Schälte <31767307+yannikschaelte@users.noreply.github.com>

* add commit numbers to release notes

Co-authored-by: Yannik Schälte <31767307+yannikschaelte@users.noreply.github.com>
@codecov-io
Copy link

codecov-io commented Mar 19, 2021

Codecov Report

❗ No coverage uploaded for pull request base (main@8f3bd49). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #122   +/-   ##
=======================================
  Coverage        ?   87.57%           
=======================================
  Files           ?        6           
  Lines           ?      475           
  Branches        ?        0           
=======================================
  Hits            ?      416           
  Misses          ?       59           
  Partials        ?        0           

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 8f3bd49...d680fdc. Read the comment docs.

@jvanhoefer
Copy link
Member Author

The problem with codacy seemed to be, that it didn't analyze the main branch due to renaming. The code quality is fine ("grade A").

@yannikschaelte
Copy link
Member

The problem with codacy seemed to be, that it didn't analyze the main branch due to renaming. The code quality is fine ("grade A").

ah, thanks for pointing out. we may need to fix this also in other repositories.

Copy link
Member

@dilpath dilpath left a comment

Choose a reason for hiding this comment

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

👍

@@ -6,7 +6,7 @@ on:

jobs:
deploy:
runs-on: ubuntu-latest
runs-on: ubuntu-18.04
Copy link
Member

Choose a reason for hiding this comment

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

@yannikschaelte Do you know if ubuntu-latest is now possible?

Copy link
Member

Choose a reason for hiding this comment

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

yes, that should work (I thought I had changed it to latest everywhere)

@@ -67,7 +67,7 @@ odes

Define **ODEs** (and states). An ODE consists of a `stateId` (string), a `rightHandSide` (string, encoding a mathematical expression), and an `initial value`. Initial values can be either numerical values or parameter Ids.

For a more detailed description of the parsing of mathematical expressions ( for `rightHandSide`) we refer to the [corresponding section](#parsing-of-mathematical-equations) of this documentation.
For a more detailed description of the parsing of mathematical expressions ( for `rightHandSide`) we refer to the :ref:`corresponding section<Parsing of mathematical equations>` of this documentation.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
For a more detailed description of the parsing of mathematical expressions ( for `rightHandSide`) we refer to the :ref:`corresponding section<Parsing of mathematical equations>` of this documentation.
For a more detailed description of the parsing of mathematical expressions (for `rightHandSide`) we refer to the :ref:`corresponding section<Parsing of mathematical equations>` of this documentation.

0.2.2 (2021-03-19)
------------------

* Add checks in SBML conversion, e.g. to catch invalid identifiers and equations (# 118).
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* Add checks in SBML conversion, e.g. to catch invalid identifiers and equations (# 118).
* Add checks in SBML conversion, e.g. to catch invalid identifiers and equations (#118).

Not familiar but it looks like it's possible to make these links to the corresponding PRs (and issues) if desired.
Above is a typo fix, below is the typo fix + what should be a working link.

Suggested change
* Add checks in SBML conversion, e.g. to catch invalid identifiers and equations (# 118).
* Add checks in SBML conversion, e.g. to catch invalid identifiers and equations ([#118][p118]).

Copy link
Member

Choose a reason for hiding this comment

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

Might be that the PR numbers in the current format will be automatically linked on the GitHub releases page anyway, so the second suggestion here would be redundant

Copy link
Member

Choose a reason for hiding this comment

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

on github, they will be linked already. on rtd in either case not, but that's ok I think.

time:
variable: t

parametersR:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
parametersR:
parameters:

Did this typo come from yaml2sbml or manually? Might be worth checking

Copy link
Member Author

Choose a reason for hiding this comment

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

That is a very good point! Thank you! I am a bit surprised, since apparently this file is never used for testing and also was (looking also at the name) never intended to test these errors. This is a copy-paste error, that I will quickly fix in another PR before we do the new release :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Did this typo come from yaml2sbml or manually? Might be worth checking

And no, fortunately copy paste error from another test case testing exactly these issues :)

noiseDistribution: normal


conditions2:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
conditions2:
conditions:

As above, might be worth checking the source of this typo.

Comment on lines +385 to +391
if math is not None:
f.setMath(math)
else:
raise RuntimeError(f'Unable to generate assignment for funtion '
f'{function_id}, libsbml can not parse the given '
f'function expression, given by '
f'lambda({arguments} , {formula}).')
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if math is not None:
f.setMath(math)
else:
raise RuntimeError(f'Unable to generate assignment for funtion '
f'{function_id}, libsbml can not parse the given '
f'function expression, given by '
f'lambda({arguments} , {formula}).')
if math is not None:
f.setMath(math)
else:
raise RuntimeError(f'Unable to generate assignment for function '
f'{function_id}, libsbml can not parse the given '
f'function expression, given by '
f'lambda({arguments} , {formula}).')

@jvanhoefer jvanhoefer mentioned this pull request Mar 19, 2021
* fix tests

* Update tests/test_yaml2sbml.py

Co-authored-by: Yannik Schälte <31767307+yannikschaelte@users.noreply.github.com>

* Update tests/test_yaml2sbml.py

Co-authored-by: Yannik Schälte <31767307+yannikschaelte@users.noreply.github.com>

Co-authored-by: Yannik Schälte <31767307+yannikschaelte@users.noreply.github.com>
@jvanhoefer jvanhoefer merged commit 6f42ee5 into main Mar 21, 2021
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.

4 participants