Skip to content

Replaced Model.set_equation(lhs, rhs) with Model.remove_equation(equation)#171

Merged
MichaelClerx merged 4 commits intodevelopfrom
118-replace-equation
Dec 6, 2019
Merged

Replaced Model.set_equation(lhs, rhs) with Model.remove_equation(equation)#171
MichaelClerx merged 4 commits intodevelopfrom
118-replace-equation

Conversation

@MichaelClerx
Copy link
Copy Markdown
Contributor

@MichaelClerx MichaelClerx commented Dec 2, 2019

Fixes #118.

As discussed in #118 I removed set_equation(lhs, rhs) and instead added remove_equation(eq) which can be used together with add_equation(eq) to achieve the same effect.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@codecov
Copy link
Copy Markdown

codecov Bot commented Dec 2, 2019

Codecov Report

Merging #171 into develop will decrease coverage by 0.03%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #171      +/-   ##
===========================================
- Coverage    95.92%   95.89%   -0.04%     
===========================================
  Files            6        6              
  Lines          883      876       -7     
  Branches       186      182       -4     
===========================================
- Hits           847      840       -7     
  Misses          19       19              
  Partials        17       17
Impacted Files Coverage Δ
cellmlmanip/model.py 99.62% <100%> (-0.01%) ⬇️

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 5997584...df0d4e0. Read the comment docs.

Copy link
Copy Markdown
Contributor

@MauriceHendrix MauriceHendrix left a comment

Choose a reason for hiding this comment

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

Looks pretty good to me.

Comment thread cellmlmanip/model.py Outdated
Comment thread cellmlmanip/model.py
Comment thread cellmlmanip/model.py
Comment thread cellmlmanip/model.py
Copy link
Copy Markdown
Contributor

@jonc125 jonc125 left a comment

Choose a reason for hiding this comment

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

Looks fine. I suggested a possible simplification, but nothing dramatic.

@skeating skeating mentioned this pull request Dec 5, 2019
27 tasks
@MichaelClerx MichaelClerx merged commit 575cb90 into develop Dec 6, 2019
@MichaelClerx MichaelClerx deleted the 118-replace-equation branch December 6, 2019 10:00
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.

Replace a variable's equation

3 participants