pyomo icon indicating copy to clipboard operation
pyomo copied to clipboard

LinearExpression should be immutable like all other expression nodes

Open michaelbynum opened this issue 3 years ago • 3 comments

Summary/Motivation:

As far as I know, LinearExpression is the only expression object in the expression system that is mutable. This PR updates LinearExpression so that it is immutable.

Legal Acknowledgement

By contributing to this software project, I have read the contribution guide and agree to the following terms and conditions for my contribution:

  1. I agree my contributions are submitted under the BSD license.
  2. I represent I am authorized to make the contributions and grant the license. If my employer has rights to intellectual property that includes these contributions, I represent that I have received permission to make contributions and grant the required license on behalf of that employer.

michaelbynum avatar Mar 04 '22 06:03 michaelbynum

Codecov Report

Merging #2330 (f1c1a0a) into main (b48c886) will decrease coverage by 0.00%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2330      +/-   ##
==========================================
- Coverage   85.61%   85.61%   -0.01%     
==========================================
  Files         618      618              
  Lines       76206    76237      +31     
==========================================
+ Hits        65241    65267      +26     
- Misses      10965    10970       +5     
Flag Coverage Δ
linux 82.43% <100.00%> (+<0.01%) :arrow_up:
osx 72.68% <100.00%> (+0.01%) :arrow_up:
other 82.38% <100.00%> (+<0.01%) :arrow_up:
win 79.47% <100.00%> (+<0.01%) :arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pyomo/core/expr/numeric_expr.py 98.34% <100.00%> (+0.05%) :arrow_up:
pyomo/common/env.py 89.15% <0.00%> (-2.01%) :arrow_down:

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 b48c886...f1c1a0a. Read the comment docs.

codecov[bot] avatar Mar 04 '22 08:03 codecov[bot]

This change needs some discussion before merging. At a minimum I think some documentation updates are needed in https://github.com/Pyomo/pyomo/blame/main/doc/OnlineDocs/advanced_topics/linearexpression.rst and maybe https://github.com/Pyomo/pyomo/blob/main/doc/OnlineDocs/developer_reference/expressions/design.rst

blnicho avatar Mar 08 '22 00:03 blnicho

@blnicho, I completely agree. I want to make sure everyone agrees with these changes before I spend time on the documentation, though.

michaelbynum avatar Mar 08 '22 21:03 michaelbynum

Addressed in another branch by @jsiirola.

mrmundt avatar Oct 04 '22 18:10 mrmundt